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

Write best practices and style guide #904

Merged
merged 38 commits into from
Sep 30, 2023

Conversation

EmilyGraceSeville7cf
Copy link
Contributor

closes #902

@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as draft May 10, 2023 22:39
@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as ready for review May 11, 2023 00:58
@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as draft May 11, 2023 01:07
@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as ready for review May 11, 2023 02:16
@kubouch
Copy link
Contributor

kubouch commented May 11, 2023

I think it's interesting to write style guide for Nushell which we can later use to develop an autoformatter, but I think this text is written too authoritatively. The "Always do this and that..." formulations leave a hostile impression on the reader. I would formulate it not as a canonical style guide that you must follow but instead as a working document collecting style recommendations. It is going to evolve as Nushell evolves, we might reach different consensus on some issues etc. Also not sure if it should be book or somewhere else (maybe cookbook?). Nevertheless, I think it's a great first step towards having a style guide, once it is formulated a bit softer. Thanks!

@EmilyGraceSeville7cf
Copy link
Contributor Author

The "Always do this and that..." formulations leave a hostile impression on the reader.

I've changed them to It's recommended to. :)

@EmilyGraceSeville7cf EmilyGraceSeville7cf force-pushed the feature/add-best-practices branch 2 times, most recently from b0ac23f to 8980677 Compare May 11, 2023 08:08
book/style_guide.md Outdated Show resolved Hide resolved
book/style_guide.md Outdated Show resolved Hide resolved
@fdncred
Copy link
Collaborator

fdncred commented May 11, 2023

@EmilySeville7cfg I'm not trying to be contentious. Thanks for putting this together. I'm betting there will be lots of opinions on what people like. It's a great conversation starter and think it's a great idea to have something like this, especially as it relates to creating a formatting tool.

book/style_guide.md Outdated Show resolved Hide resolved
Co-authored-by: Darren Schroeder <[email protected]>
@EmilyGraceSeville7cf
Copy link
Contributor Author

Any updates? :)

@amtoine
Copy link
Member

amtoine commented May 26, 2023

i think we can merge this 👍

@fdncred
are we good? 😋

@fdncred
Copy link
Collaborator

fdncred commented May 26, 2023

I'm not signing off that all the styles documented here are what we want to move forward with, especially since I doubt JT and others have even looked at this PR. However, I think we could probably land it, point other core-team members to this link, and continue working on it.

@amtoine
Copy link
Member

amtoine commented May 26, 2023

yup agree 👍

the idea was to avoid letting this PR die / live too long 😌

@AucaCoyan AucaCoyan mentioned this pull request Jun 2, 2023
22 tasks
@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Jun 17, 2023

I am here again. ;) I see this PR is referenced in nushell/nufmt#11. Any extra updates about my style guide?

@fdncred
Copy link
Collaborator

fdncred commented Jun 17, 2023

Hey @EmilySeville7cfg! Welcome back. Hope you had a good trip and are ready to jump back in.

I was just about to land this when I realized that I think it'll be hidden. If we intend this to be a chapter in the book, we need to have a link to it like all the other chapters are linked. I think it would best fit under "Programming in Nu".

If we can finish updating the contributor-book, maybe a better place is to have it there? We can move it later if we want to.

@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Jun 17, 2023

If we can finish updating the contributor-book, maybe a better place is to have it there?

Why not? :)

@fdncred
Copy link
Collaborator

fdncred commented Jun 17, 2023

Why not? :)

It just takes someone updating all the chapters (except plugins, which just got updated). If you want to do that, we can land this PR and then move it there when the contributor-book is finished?

@EmilyGraceSeville7cf
Copy link
Contributor Author

If you want to do that, we can land this PR and then move it there when the contributor-book is finished?

I want it. It's not clear for me now how exactly to update chapters. Refreshing code samples according to this style guide and checking whether everything is up to date?

@fdncred
Copy link
Collaborator

fdncred commented Jun 17, 2023

If you want to do that, we can land this PR and then move it there when the contributor-book is finished?

I want it. It's not clear for me now how exactly to update chapters. Refreshing code samples according to this style guide and checking whether everything is up to date?

I think you just need to go to this link and update the markdown files. https://github.com/nushell/nushell.github.io/tree/main/contributor-book. I haven't looked at them in a while but the things that don't work in nushell anymore need to be updated. I think you understand that part. We'll have to take it chapter by chapter probably.

@savente93
Copy link
Contributor

Not sure if this is the correct place for it, I'll leave that up to the author/reviewers, but it would also be nice to add some style guides for contributing to the book. For example:

  1. Code examples should use the nu formatting as in ```nu , currently this is not very consistent as I see none, bash, sh, nu and nushell used. I think it would be nice to have these consistent
  2. Another that might be nice is to use a consistent style for output formating of tables. E.g. "examples of table output should use rounded corners" (don't actually mean it needs to be rounded, just that we would use a consistent styling"

I'm sure other ideas can be added but these are just a few that I came across

@fdncred
Copy link
Collaborator

fdncred commented Sep 30, 2023

I think we should probably merge this. I'm not sure it's 100% right but it's more than we have.

Related to @savente93's comment. We should have that too, but I'm not sure where. We should definitely make it clear how to contribute.

@savente93
Copy link
Contributor

I'll make a seperate issue for it then

@fdncred
Copy link
Collaborator

fdncred commented Sep 30, 2023

Let's move forward with this. If we end up not liking something, we can change it.

@fdncred fdncred merged commit c6e0eb9 into nushell:main Sep 30, 2023
2 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.

Write code style guide
5 participants