-
-
Notifications
You must be signed in to change notification settings - Fork 7
New page: Packaging/Workflow/Submitting-a-PR #66
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
base: main
Are you sure you want to change the base?
Conversation
Add a new page to provide guidance on how AerynOS would like PR's submitted to it's repositories. Mainly focused on the recipes repo however I've tried to make it generic as well. Utilised current Solus and old Solus-Project documentation as guidance.
ermo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good start.
I think we need to mention active, imperative voice and then find a recent example where we've actually followed the guidance herein. *ahem*
| Give a very brief summary in the first line of your commit message, as a very very high level overview of what this change is achieving. Then write another paragraph (doesn't need to be a story) describing the rationale and results of the change itself. | ||
|
|
||
| Git users may be viewing your changes on the terminal, so please respect the 80x24 rules. | ||
| Try not to wrap your lines, rather, manually line break them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this means.
Summary: Add an aside to highlight AerynOS' requirement to use the immperative mood for git commits
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
|
Couple of comments after reading the entire thing: Perhaps not for this PR but the other one we want people to add into a recipe why a specific flag was added if it deviates from the expected macros. I'm not sure if the line length matters personally, Is that really something we wish to enforce? Perhaps if it's excessive but I think most proper tools would already wrap it at that point like it does here. |
Updating-an-existing-recipe page
ermo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions.
| - Test plan demonstrating that you have actually confirmed the changes work on your local system | ||
| - If the change resolves an issue, include a Resolves line with the issue number (Where `issuenumber` is the issue number of the package request/update). | ||
|
|
||
| The last point about the test plan is particularly important, as it ensures that the changes have been thoroughly tested and verified before being merged into the main codebase. There is an explicit agreement that you take ownership of the quality of the code you submit and understand that if there are issues, you are likely to be the first person asked to fix an issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The last point about the test plan is particularly important, as it ensures that the changes have been thoroughly tested and verified before being merged into the main codebase. There is an explicit agreement that you take ownership of the quality of the code you submit and understand that if there are issues, you are likely to be the first person asked to fix an issue. | |
| The last point about the test plan is particularly important, as it ensures that the changes have been tested and verified before being merged into the main codebase. There is an explicit agreement that you take ownership of the quality of the recipe updates you submit, and that you understand that if there are issues, you are likely to be the first person consulted to fix said issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thought process here was whilst it's current very recipe repo specific, submitting a PR could apply to all repositories so I tried to use language that could also apply elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thought process here was whilst it's current very recipe repo specific, submitting a PR could apply to all repositories so I tried to use language that could also apply elsewhere?
Co-authored-by: Rune Morling <ermo@serpentos.com>
Co-authored-by: Rune Morling <ermo@serpentos.com>
Co-authored-by: Rune Morling <ermo@serpentos.com>
Summary
New page for guidance on how to submit a PR to AerynOS; mainly focused on recipes repo but also somewhat generic in guidance:
Test plan
Success