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

main: rework usage #4467

Merged
merged 2 commits into from
Sep 28, 2024
Merged

main: rework usage #4467

merged 2 commits into from
Sep 28, 2024

Conversation

leongross
Copy link
Contributor

Remove unused switch case and improve/add command-specific usage from website.

remove unused switch case statement and improve/add command specific usage.

Signed-off-by: leongross <leon.gross@9elements.com>
@leongross
Copy link
Contributor Author

From a cluttering perspective, I am unsure if we want to keep all the static usage messages in the middle of the code or refactor it somewhere else, something like const.go holding all that static data?

@dkegel-fastly
Copy link
Contributor

The message is significantly longer now.... I wonder if we want the old version as the default, and the new one as "more help" somehow?

@leongross
Copy link
Contributor Author

leongross commented Sep 14, 2024

The old version is still there. I aligned the usage behavior to big go.
tinygo help displays the old message.tinygo help <subcommand> shows the additive help if there is any.

@leongross
Copy link
Contributor Author

So with that in mind, what do you think? @dankegel @dkegel-fastly @deadprogram

@leongross
Copy link
Contributor Author

@deadprogram what do you think of this?

@leongross
Copy link
Contributor Author

If there are no change requests, can we merge this? @dankegel @archie2x @dkegel-fastly @dgryski

Copy link
Member

@dgryski dgryski left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@archie2x archie2x left a comment

Choose a reason for hiding this comment

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

I have have nits about the wording of things but more accessible documentation is great.

main.go Outdated Show resolved Hide resolved
Copy link
Member

@dgryski dgryski left a comment

Choose a reason for hiding this comment

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

Just noticed in my editor (not visible on github diff view) that there are a few lines with trailing space that should probably be cleaned up.

* limit help to 80 col width
* remove inconsistent ':'
* remove trailing spaces
* rmove spaces woth tabs
* reworkd build command

Signed-off-by: leongross <leon.gross@9elements.com>
@aykevl
Copy link
Member

aykevl commented Sep 28, 2024

I think this is an excellent idea! So if nobody objects, I'd like to see this merged :)
The only thing I'm worried about is keeping it in sync with the documentation at tinygo.org. I'm afraid we're going to forget updating the documentation in the code when we update the website.

@deadprogram
Copy link
Member

@dgryski I think your change request was already made, no?

@leongross
Copy link
Contributor Author

@aykevl keeping it in sync is also a problem I see. First, I think adding this for now is a big improvement to the current state. In the long run it mayb be a good idea to automatically pull this from the website source on a regular basis. I thought how to do this but since the webiste uses html amd css styling this is not trivial to convert.

Copy link
Member

@dgryski dgryski left a comment

Choose a reason for hiding this comment

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

LGTM

@deadprogram
Copy link
Member

Thank you very much for working on this @leongross and to @dgryski @aykevl @archie2x @dkegel-fastly for reviews. Now squash/merging.

@deadprogram deadprogram merged commit 52788e5 into tinygo-org:dev Sep 28, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants