-
Notifications
You must be signed in to change notification settings - Fork 79
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
Use DEP-14 branch names debian/latest
and upstream/latest
#225
Conversation
Please allow the CI to run so I can see the result, and give me enough time to fix all errors :) |
It looks like CI did run when you submitted the PR, am I missing something? |
After submission there was a banner stating that "user does not have permissions to trigger the CI" and that my account must be allowlisted. Later seems the CI ran, not sure if somebody did something or not. |
I did 😄 |
I tested this manually by running I also added further refinements based on my Debian packaging best practices knowledge. I am even tempted to add a generic Let me know if you want me to split this into multiple separate PRs. |
e354493
to
e2bfbec
Compare
Thanks for the review @n-peugnet. Add remarks are resolved and branch updated. I also have test binaries available at https://salsa.debian.org/otto/dh-make-golang/-/jobs/6641396/artifacts/browse/debian/output/ in case you want to test how this would behave eventually when the dh-make-golang package in Debian gets updated. |
Example output when trying to package Glow:
|
Can you add me to the Debian org on GitHub so GitHub knows I am to be trusted? |
From what I understand, some of these change should first be proposed to the team as a whole, since the branch names have already been updated in the past, for example: https://go-team.pages.debian.net/workflow-changes.html About the removal of the |
I split this out into #230. |
ef5745f
to
8253f23
Compare
Seems not all are aware of DEP-14 contents, so I'll re-iterate here the relevant part: Go team policy in 2017 switched to use DEP-14:
In 2017 DEP-14 was still being changed, and later the Debian-wide agreement was that the branch names should be https://dep-team.pages.debian.net/deps/dep14/
This proposal is simply updating the implementation, while the intent of the policy stays the same. |
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've added a few comments and one issue I see with the code changes.
6155540
to
c40d7cb
Compare
@penguin359 Would you like to test this PR? Easy steps to do so:
|
I ran
The import works correctly, including creating the pristine-tar branch and merging with full audit trail release git tag -> upstream/latest import branch -> copyright Filter application -> debian/latest. |
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.
Just one stylistic nitpick, but otherwise - looks good to me!
I ran through the test you provided. I did notice a few things, however, I suspect all of these are from upstream, not your PR. I'm just going to document it here for the moment. One item I noticed was a big, fat warning from Git about the initial branch name defaulting to master because "init.defaultBranch" was not set in my container environment. Adding that as a I also saw a message about "Deleting upstream vendor/ directories", however, it appears that the Also, I haven't confirmed it yet, but it seems like it's signing upstream tags on |
Yes indeed,
->
I actually did this by simply deleting code, as this solution was already envisioned by @elboulangero in 729d8db in 2021.
I am not sure what this is without a copy-paster of full output, but I assume this was git-buildpackage doing it on branch
Seems this code isn't running uscan at all:
->
The final argument |
In DEP-14, the preferred branch name for the Debian packaging target branch is `debian/latest` and the preferred name for the upstream import target branch is `upstream/latest`. Note that the upstream development branch name can be whatever and should stay as it is upstream, typically `main` or `master`. The branch `upstream/latest` should not point to the latest upstream development commit, but to the latest commit that was used as the upstream release that the Debian revision was derived from.
Instead of using various different upstream remote names, use the one and same upstream git remote name consistently. As the name pick `upstreamvcs` just as git-buildpackage does, so that if anybody runs `gbp clone` they will automatically end up with the same git remotes and branches as anyone in to go-team.
When creating a new package, populate the git-buildpackage with additional configs and in-line comments on why and how to use them. This will make go packaging easier, more consistent and more secure as the best practices flow to all packages via good defaults. Also add comment to explain why pristine-tar is beneficial.
Debian 11 "Bullseye" shipped with Git 2.30, so it is unlikely anybody doing Debian packaging uses a Git version older than that anymore, at least not in combination with this latest dh-make-golang version. Thus it is possible to activate the shorthand git init form as planned in 2021 since 729d8db.
Two weeks passed since I pushed new version and responded to comments. I marked the items I fixed now as resolved as well and will proceed to merge this soon. |
In DEP-14, the preferred branch name for the Debian packaging target branch is
debian/latest
and the preferred name for the upstream import target branch isupstream/latest
. Note that the upstream development branch name can be whatever and should stay as it is upstream, typicallymain
ormaster
. The branchupstream/latest
should not point to the latest upstream development commit, but to the latest commit that was used as the upstream release that the Debian revision was derived from.