Code review comment for ~ubuntu-desktop/ubuntu/+source/gnome-initial-setup:enable-location-page

Revision history for this message
Sebastien Bacher (seb128) wrote :

> Your commit doesn't apply cleanly since ubuntu/master has a debian/changelog entry that I guess isn't
> in your fork.

Sure, there were other pending branches and I merged one. Resolving changelog conflicts is common for merges and not really something that needs to be picked about in reviews imho

> It doesn't build because you added a line to the patch so you'll need to bump the -82,6 +85,16 line to
> "85,17" or just use the gbp pq workflow to edit patches.

Thanks for catching that up. I used quilt to refresh the patch but that added formatting changes so I reverted those and reverted too much. The gbp pq workflow is too much hassle, I just went for changing the 16 to 17

> Your commit message doesn't really follow usual git practice. I guess you used debcommit? I consider
> this a nitpick (so not essential to fix) since it's in the Ubuntu-specific part of the package, but I
> think you'd want to use more conforming commit messages when pushing commits to Debian.

Yes, I used "debcommit -e" which use the changelog entry. I don't know what the "usual git practice" is so I'm letting like that, we have standard tools (debcommit) so we should get those to be fixed/working in an acceptable way.

« Back to merge proposal