Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple OS tabs for dev setup instructions #55

Merged

Conversation

petejohanson
Copy link
Contributor

Closes #12 and #26

@petejohanson petejohanson added the documentation Improvements or additions to documentation label Jul 14, 2020
@petejohanson petejohanson requested review from Nicell and innovaker July 14, 2020 10:46
@petejohanson petejohanson self-assigned this Jul 14, 2020
@petejohanson
Copy link
Contributor Author

Direct link to the deploy for this docs page: https://deploy-preview-55--zmk.netlify.app/docs/dev-setup

Copy link
Member

@Nicell Nicell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things to comment on.

Would be nice if there were a bounding box of some sort around the parts that are OS (or shell) specific. It's a little jarring switching between them without that IMO. I can look into this.

Would also be great if clicking between them when scrolled down didn't make your screen shift a bunch. This might be able to be fixed by making the page anchor to the button you click... Maybe.

Last thing is it would be cool if cmd.exe was tied to the Windows tab. The rest can go to bash I would say. Maybe that's a little awkward with the different number of tabbed items etc.

Otherwise this looks great! A huge improvement.

@petejohanson
Copy link
Contributor Author

@Nicell Thanks for the review. Responses inline:

Would be nice if there were a bounding box of some sort around the parts that are OS (or shell) specific. It's a little jarring switching between them without that IMO. I can look into this.

Yeah, I noticed this as well. I do think a bounding box would help. It's how the Zephyr docs handle this. Might be worth pulling the latest Docusaurus V2 stuff to see if their upstream theme has fixed this, before trying to implement ourselves. I don't really think we should block merging this until that's resolved, but what are your thoughts?

Would also be great if clicking between them when scrolled down didn't make your screen shift a bunch. This might be able to be fixed by making the page anchor to the button you click... Maybe.

Yeah. Another pain. My hope is that most folks will just pick one from the first one, and then the fact that they are synced will mean they won't be switching the others further down the page.

Last thing is it would be cool if cmd.exe was tied to the Windows tab. The rest can go to bash I would say. Maybe that's a little awkward with the different number of tabbed items etc.

Yeah. A nice to have. Doing so would require us to ditch the upstream Tabs/TabItem stuff and roll our own. Perhaps a future enhancement.

@Nicell
Copy link
Member

Nicell commented Jul 14, 2020

@petejohanson These were all nitpicks, and I think this is definitely ready to merge. The biggest thing is the bounding box stuff. Hopefully we can figure something like that out soon.

As for the cmd.exe coming soon for getting env variables to load every time, I left that out as I couldn't find docs for that in relation the Windows. We may need to come up with our own way. In fact, the sourcing .cmd file doesn't actually seem to do anything, so Zephyr may already set everything properly on Windows during the prior installation steps. I need to look into it more.

@petejohanson petejohanson merged commit c5b4503 into zmkfirmware:main Jul 14, 2020
MangoIV referenced this pull request in MangoIV/zmk Dec 18, 2020
Multiple OS tabs for dev setup instructions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Development Basic Setup on Raspberry Pi OS (Buster)
2 participants