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

new package: Update example to subject: verb version #496

Merged
merged 5 commits into from
Mar 1, 2024

Conversation

ermo
Copy link
Contributor

@ermo ermo commented Feb 28, 2024

Description

With the monorepo, we use the worklog.py script and the GH UI, which shows the git summary for each commit at a glance.

Therefore, the git summaries in commits should be as informative as possible so people don't have to go chasing the individual commit logs for relevant version information when looking for specific updates or summarising the weekly development churn in the forum sync notes.

This PR changes the old (and no longer useful) "Initial inclusion" git summary part of the commit message to a more useful "packagename: Add at version" summary.

Note that we already require "packagename: Update to version" for the same reason. This PR also expands on this by adding a section with other useful git summary commit message examples related to common package update scenarios.

With the monorepo, we use the `worklog.py` script and the GH UI, which
shows the git shortlog at a glance.

Therefore, the git shortlog messages should be as informative as possible
so people don't have to go chasing the individual commit logs for
relevant version information when looking for specific updates or
summarising the weekly development churn in the forum sync notes.

This commit changes the old (and no longer useful) "Initial inclusion"
git shortlog message to a more useful "packagename: Add at version"
git shortlog message.

Note that we already require "packagename: Update to version" for the
same reason.

Signed-off-by: Rune Morling <ermo@serpentos.com>
@ermo ermo force-pushed the noun-verb-version-commit-logs branch 3 times, most recently from 8fffcbf to 274e39e Compare February 28, 2024 13:29
The reasoning is the same as for the new packages.

The git shortlog examples shown in this in this commit have already
been proven useful in the context of the serpent recipe monorepo.

Signed-off-by: Rune Morling <ermo@serpentos.com>
@ermo ermo force-pushed the noun-verb-version-commit-logs branch from 274e39e to 20f2b54 Compare February 28, 2024 13:31
docs/packaging/updating-an-existing-package.md Outdated Show resolved Hide resolved
docs/packaging/updating-an-existing-package.md Outdated Show resolved Hide resolved
@ermo ermo requested a review from EbonJaeger February 28, 2024 17:39
Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

This is a good start. I think we can improve it further, though. :)

docs/packaging/updating-an-existing-package.md Outdated Show resolved Hide resolved
Copy link
Member

@davidjharder davidjharder left a comment

Choose a reason for hiding this comment

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

I don't have much to add here.

  • I agree with the direction Evan wants to go.
  • I would advise against the term shortlog, we don't use it anywhere. I know "title" and "description" are not the correct terms for what is actually all part of the commit message, but people will understand them more intuitively, and github uses those terms.
  • We should not link to "tooling central documentation", a repository our users are not familiar with and we do not control. We could steal that guidance and put it on our Git practices page later

A more heretical bit of advice would be to fix exactly one line with this PR to address the precise issue that kicked this all off. I would be fine with that. Your call ermo. The longer this PR is open, the more changes will suggest themselves

@TraceyC77
Copy link
Contributor

I agree with David's points.

@ermo
Copy link
Contributor Author

ermo commented Mar 1, 2024

@davidjharder , @TraceyC77 , @EbonJaeger

Thank you all for your (excellent!) suggestions.

I've essentially achieved what I wanted with this, which is to shine a light on how the workflow and examples we show new (and returning) packagers will help make the monorepo commit shortlogs more useful to packagers (including Solus Staff) overall.

I have my hands full with Serpent now coming properly online and simply can't afford to give this the copy-editing time it needs in order to be of high enough quality to meet your standards AND be easy to read.

Therefore, I am of the opinion that this really ought to be taken over by the comms/doc team, as you are clearly better at judging what users need here?

Feel free to close this and use it as a basis for a better worded PR?

EDIT: As a show of good faith, I'm giving this a final cycle where I address the review comments to the best of my ability. After that, I nominate @EbonJaeger to drive it over the finish line so it ends up in the best possible place for users and Solus as a project alike.

ermo added 2 commits March 1, 2024 12:34
Signed-off-by: Rune Morling <ermo@serpentos.com>
Signed-off-by: Rune Morling <ermo@serpentos.com>
Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

I think this is almost there! This structure is much better and easier for the reader to get the specific information they want.

docs/packaging/updating-an-existing-package.md Outdated Show resolved Hide resolved
docs/packaging/updating-an-existing-package.md Outdated Show resolved Hide resolved
Signed-off-by: Rune Morling <ermo@serpentos.com>
Copy link
Member

@EbonJaeger EbonJaeger 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 no further changes for this PR. Thanks for making this section better!

Regarding @davidjharder 's point about linking to the tooling center docs, I think that's out of scope for this PR and can be addressed by someone later on; the link was already present before this was opened.

@davidjharder davidjharder merged commit 106fee5 into master Mar 1, 2024
1 check passed
@davidjharder davidjharder deleted the noun-verb-version-commit-logs branch March 1, 2024 18:52
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.

5 participants