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

doc: GRASS Programming Style Guide #3569

Merged
merged 10 commits into from
May 20, 2024
Merged

Conversation

petrasovaa
Copy link
Contributor

This is an attempt to consolidate and update documentation for code contributors into a style guide:

  • The idea is to focus on style and best practices, contributing through GitHub is not described here.
  • The source are the files from submitting folder (originally converted from Trac wiki), I tried to extract useful and relevant things, this PR removes those files.
  • Another source is @wenzeslaus' workshop on development and best practices
  • I kept it as one document right now, but I am open to splitting it up or restructuring it.

I would appreciate any feedback on this. For example I don't have a lot about pylint usage there or best practices in C.

@petrasovaa petrasovaa self-assigned this Apr 8, 2024
@github-actions github-actions bot added the docs label Apr 8, 2024
@echoix
Copy link
Member

echoix commented Apr 8, 2024 via email

veroandreo
veroandreo previously approved these changes Apr 11, 2024
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

Looks good to me, it covers all aspects simply and concisely including examples :)

@petrasovaa petrasovaa requested a review from wenzeslaus April 26, 2024 19:59
@wenzeslaus wenzeslaus added this to the 8.5.0 milestone Apr 26, 2024
@echoix
Copy link
Member

echoix commented May 8, 2024

If I recall correctly, there is a special handling for a file named CONTRIBUTING.md, a bit like README files. If we really want to use a file in the doc subfolder, we could create a contributing file and link to the full docs in it.

The docs are here: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors

There are multiple paths that GitHub's UI will integrate these contribution guidelines on PRs for new users, or for other users if that document changes since their last contribution.

I think we should take advantage of it instead of inventing our own and being left to make it discoverable by our own too.

Our page looks like this:

https://github.com/osgeo/grass/contribute

image
image
image

@petrasovaa
Copy link
Contributor Author

If I recall correctly, there is a special handling for a file named CONTRIBUTING.md, a bit like README files. If we really want to use a file in the doc subfolder, we could create a contributing file and link to the full docs in it.

The docs are here: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors

There are multiple paths that GitHub's UI will integrate these contribution guidelines on PRs for new users, or for other users if that document changes since their last contribution.

I think we should take advantage of it instead of inventing our own and being left to make it discoverable by our own too.

The idea with this style guide is to link it from the CONTRIBUTING.md file. That file needs some improvement, but that would be a different PR.

@echoix
Copy link
Member

echoix commented May 9, 2024

Perfect then

@neteler
Copy link
Member

neteler commented May 9, 2024

IMHO this should go into G8.4.0, no need to postpone. A nice document!

@wenzeslaus
Copy link
Member

This is not published as part of the documentation, so I don't know where this will be visible in the release except release notes (which is nice), but we need this to be in the code for GSoC so we don't have to link a file in a PR.

@petrasovaa petrasovaa requested a review from wenzeslaus May 13, 2024 14:58
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all the issues.

@petrasovaa petrasovaa merged commit c189b1f into OSGeo:main May 20, 2024
23 checks passed
@petrasovaa petrasovaa deleted the style-guide branch May 20, 2024 14:44
@neteler neteler modified the milestones: 8.5.0, 8.4.0 May 20, 2024
@a0x8o a0x8o mentioned this pull request Jun 3, 2024
@mshukuno
Copy link
Contributor

@petrasovaa Thank you for creating the GitHub guide. I am sure it will be helpful to someone like me who uses GitHub but has no experience with team workflows. This "pre-commit" feature is awesome!!

I encountered a problem when I ran pre-commit run black --all-files. I got a message "No hook with id black in stage 'pre-commit" message. I believe the hook id is black-jupyter in pre-commit-config.yaml.

@echoix
Copy link
Member

echoix commented Jun 24, 2024

I encountered a problem when I ran pre-commit run black --all-files. I got a message "No hook with id black in stage 'pre-commit" message. I believe the hook id is black-jupyter in pre-commit-config.yaml.

Yep, you are completely right, it was changed a couple months ago. It is the pre-commit equivalent of installing black with jupyter support with pip install black[jupyter] instead of pip install black. (With pip, the brackets are called "extras" and are optional features, delimited by commas, to also install optional packages that enable optional features).

If you want to run on all files, not only files that you are about to commit, I usually use

pip install pre-commit
pre-commit run -a

And that installs and runs on all files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

7 participants