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

Fix 107 #153

Merged
merged 3 commits into from
Aug 26, 2021
Merged

Fix 107 #153

merged 3 commits into from
Aug 26, 2021

Conversation

elboulangero
Copy link
Contributor

@elboulangero elboulangero commented Jan 19, 2021

Refer to #107 for the reason why. I believe this fix will be helpful for newcomers.

This minor refactoring prepares the code for the next commit, where we
will need to know the Debian branch in `execMake()`.
@elboulangero
Copy link
Contributor Author

Anyone willing to review this one?

@creekorful creekorful self-requested a review August 2, 2021 21:55
make.go Outdated Show resolved Hide resolved
elboulangero and others added 2 commits August 3, 2021 09:34
As mentiond in Debian#107, running 'gbp push' when there's no debian tag is
not the right thing to do,  as it will only push the upstream branch.
Consequences are:

1. The default gitlab branch is set to the upstream branch instead of
   the debian branch.
2. The debian branch is not pushed and needs to be pushed manually
   anyway.

I believe that the best thing to do instead is just to run 'git push
origin <debian-branch>' first, and then run 'gbp push'.

Closes: Debian#107
Thanks to Alois Micard for pointing that out
@@ -991,6 +991,7 @@ func execMake(args []string, usage func()) {
fmt.Printf(" dh-make-golang create-salsa-project %s\n", debsrc)
fmt.Printf("\n")
fmt.Printf("Once you are happy with your packaging, push it to salsa using:\n")
fmt.Printf(" git push origin %s\n", debBranch)
fmt.Printf(" gbp push\n")
Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me, however I'm still not sure about gbp push. As per the man page and the bug report, it is clearly indicated that gbp push is intended to be run after uploading a Debian package to the archive. In the bug report Guido suggest simply running git push when there was no release.

Adding a --tips flag to gbp push has been suggested here but looks not implemented atm.

Personally, I'm doing a git push --all && git push --tags in order to push all the branches and the tags to the Salsa repository. I wonder what the rest of the team think about that. @zhsj any opinion on this?

Copy link
Member

Choose a reason for hiding this comment

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

It's clear that the gbp push workflow doesn't work here. However I would much prefer implementing a dh-make-golang push command. And I don't like git push --tags since it pushes all upstream tags which are not needed, for example upstream have v1.0.0 -> v10.0.0 tags, while we only need upstream/10.0.0 tag.

Copy link
Member

Choose a reason for hiding this comment

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

Ack, so this dh-make-golang push command should be running a git push origin to push the branches and then push all the tags beginning by upstream/ ?

Copy link
Member

Choose a reason for hiding this comment

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

Probably like gbp push, but allow to push wip debian branch:

  • git push origin debian/sid
  • git push origin upstream
  • git push origin upstream/<tag>
  • (optional) git push origin pristine-tar
  • (optional) git push origin debian/<tag>

While I'm writing above, I find it forces dh-make-golang to parse gbp.conf to see the upstream branch name ..

Copy link
Member

Choose a reason for hiding this comment

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

Great ideas everyone! I like how @elboulangero has improved the code too. As I am preparing for a v0.5.0 release today, I think I will go ahead and merge this PR now while @zhsj continues to work on dh-make-golang push. Many thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks all for the review and feedback!

@creekorful creekorful self-requested a review August 3, 2021 05:48
@anthonyfok anthonyfok merged commit 729d8db into Debian:master Aug 26, 2021
@anthonyfok anthonyfok added this to the v0.5.0 milestone Aug 30, 2021
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.

4 participants