docs: add contribution section, revise explanations#6085
docs: add contribution section, revise explanations#6085medubelko wants to merge 3 commits intocanonical:mainfrom
Conversation
- Rename and rewrite 'Contribute to this documentation'. - Add 'Contribute to development'. - Add 'About this documentation'. - Add second toctree.
medubelko
left a comment
There was a problem hiding this comment.
@jahn-junior @lengau @mr-cal @bepri This isn't quite finished but it's more than a draft. In the interest of time, I'd like you to take a look the way this is organised and structured (pages, headings, order) and see how you feel about it. The main details are the same as in the existing CONTRIBUTING.md, so there shouldn't be any surprises.
@asanvaq Rockcraft shares the same governance and process, so its contribution pages ought to have the same shape as this. Please use this as a template if you'd like to get started.
| ``issue-235-add-string-sanitizer``. | ||
|
|
||
|
|
||
| Develop |
| Snapcraft is amenable to code generated with LLM assistance. Generating code is a time | ||
| saver, but like all code it needs testing and careful review. An LLM doesn't absolve you | ||
| of responsibility -- you are ultimately responsible for the code in your work. |
There was a problem hiding this comment.
I expect many revisions will come to this, so please consider this a best first effort.
In the medium-term, I think the simplest way to cut through the friction is to add a todo item for LLM disclosure to the PR template.
|
|
||
| When combining commit types, select the highest-ranked type that fits: | ||
|
|
||
| - :vale-ignore:`ci` |
There was a problem hiding this comment.
Not working because of an upstream bug.
There was a problem hiding this comment.
| working on snaps. | ||
|
|
||
|
|
||
| Bounties |
There was a problem hiding this comment.
I think we can drop this, given that we're sunsetting the bounty program.
|
|
||
| make test | ||
|
|
||
| Running all tests can take a very long time, in some cases an hour. |
There was a problem hiding this comment.
This PR is landing on main and we'll be removing tests/legacy/unit on main next month. The time to run make test is going to drop to a few minutes or less (55 seconds on my system), so this notice may not be needed.
| Commit early and often. It's normal to make multiple commits for a single piece of work, | ||
| especially when you come back to review it later. It's a good practice to get into to | ||
| keep your changes safe. | ||
|
|
||
| Committing triggers the pre-commit hook, which runs autoformatters. If any files were | ||
| autoformatted, re-add them and redo the commit. |
There was a problem hiding this comment.
This is slightly different than the help in the develop section and I think I prefer this version of it better. Unless you intentionally made them different?
| after. **After the PR is approved, there may be a delay before merge.** The maintainers | ||
| might need time to coordinate the PR with other development on the project. | ||
|
|
||
| After approval, **don't** force-push to your branch. It's difficult for the maintainers |
There was a problem hiding this comment.
I like the emphasis here too, any reason not to put it in the dev page?
| .. tab-item:: With HTTPS | ||
|
|
||
| If you don't authenticate with SSH, clone with `HTTPS | ||
| <https://docs.github.com/en/get-started/git-basics/about-remote-repositories#cloning-with-https-urls>`_ |
There was a problem hiding this comment.
| <https://docs.github.com/en/get-started/git-basics/about-remote-repositories#cloning-with-https-urls>`_ | |
| <https://docs.github.com/en/get-started/git-basics/about-remote-repositories#cloning-with-https-urls>`__ |
| .. tab-item:: With SSH | ||
|
|
||
| If you authenticate your GitHub account with `SSH | ||
| <https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account>`_, |
There was a problem hiding this comment.
| <https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account>`_, | |
| <https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account>`__, |
| Snapcraft has 10-year long-term support commitments. All changes must be | ||
| backward-compatible unless stated otherwise in the issue. | ||
|
|
||
| All nontrivial changes must accompanied by new or updated tests. The Snapcraft test |
There was a problem hiding this comment.
| All nontrivial changes must accompanied by new or updated tests. The Snapcraft test | |
| All nontrivial changes must be accompanied by new or updated tests. The Snapcraft test |
|
|
||
| .. code-block:: bash | ||
|
|
||
| snap install snapcraft_<instance> --channel <channel-name> |
There was a problem hiding this comment.
This may warrant a link to https://snapcraft.io/docs/explanation/how-snaps-work/parallel-installs/, since it will fail if the user hasn't configured snapd.
| .. code-block:: bash | ||
|
|
||
| git fetch upstream | ||
| git switch -c <new-branch-nameupstream/hotfix/<current-release> |
There was a problem hiding this comment.
| git switch -c <new-branch-nameupstream/hotfix/<current-release> | |
| git switch -c <new-branch-name> upstream/hotfix/<current-release> |
| Snapcraft has 10-year long-term support commitments. All changes must be | ||
| backward-compatible unless stated otherwise in the issue. |
There was a problem hiding this comment.
What do you think about clarifying our commitment is per-base? Or that we can't make changes that require users to update their snaps for stable bases?
Here's a great example of the expectation I'd like to document: #5514 (comment)
make lint && make test.- [ ] I've updated the relevant release notes.