Hi Nicholas, Good to hear from you! Please see my replies below.
Nicholas D Steeves <s...@debian.org> writes: > Xiyue Deng <manp...@gmail.com> writes: > >> Nicholas D Steeves <s...@debian.org> writes: >>> >>> Sorry I didn't ask this sooner, but would you prefer if I call you Deng, >>> or Xiyue, or something else? Conventions and understanding vary a lot >>> from place to place, after all. >> >> No worries! My first name is Xiyue, but I acknowledge that this is >> probably difficult to pronounce in non-Asian countries or even outside >> of China, so feel free to call me Deng, or even my code name "manphiz" >> :) > > What do you prefer? > `manphiz' is fine as it's easier to find me on IRC with that. >>> Xiyue Deng <manp...@gmail.com> writes: >>> >>> Onto the review: >>> >>>>>>> * New upstream release >>> >>> Push the upstream tag to salsa, and find a way to mitigate this issue in >>> the future. >>> >> >> Thanks for pointing this out, and this is something that confuses me. >> According to the dgit-maint-merge(7) workflow, one should have a >> upstream branch tracking upstream git repo directly, so that when you >> merge a tagged release "git deborig" can directly use upstream tags to >> create the tarball. On the other hand, if we have salsa CI set up there >> is no upstream tag on salsa so it probably will fail at "git deborig" >> stage. Still, if I read the dgit-maint-merge workflow correctly (I >> could be wrong), it only requires a "upstream/%(version)s" tag when the >> upstream only releases tarballs or when we want to package a snapshot. >> So I'm not sure whether we always want to have "upstream/%(version)s" >> tags. >> Would like to hear your opinion on this. >> > > Do you really think that correctly indicating the provenance of upstream > source is a question of "opinion"? This is the sort of comment that > indicates that I'm wasting my time reviewing your work... > > If you If you create an upstream tag then you need to name it so it's clear > that upstream didn't create it; gbp does this using a configurable > format, and 'gbp push' pushes the tag along with everything else. I > don't care what workflow you use, but you need to insure that upstream > tags are pushed. It's been a while since my last mail, and I'd like to admit that my understand of a Salsa repository vs upstream repository back then was incorrect: if we want to include upstream repository data in any form (e.g. in upstream branch, or dig-main-merge workflow), the Salsa repository should be a super-set of the upstream repo, which means it should include everything in the upstream repo (including tags) and have Debian specific changes on top of that. I have fixed it and have pushed all tags. > Also, as stated before, I won't touch anything dgit-related. > No worries. Over the past few months I realized that dgit is compatible with gbp layout, so that the way it works is compatible both ways. I have also added a `debian/gbp.conf' matching the current practice at [1][2]. >>> I'm happy to see this clear, concise, and useful phrasing. If you have >>> any pending not-yet-uploaded work that doesn't use this, please update >>> it. If you're interested in a nitpick, the key term is "substitution >>> strings" and not "[special] substitute strings" (see the manpages for >>> uscan and deb-substvars as well as codesearch.debian.net). >>> >> >> Ack. Dropping the "special" part in changelog[4]. > > Thanks :) > >>>>>>> * Fix issues in d/copyright >>>>>>> - Clarify license to be GPL-3+ to be consistent with upstream >>> >>> This is unclear. Which licence was it before, and whose license are you >>> talking about? Web-mode is a non-native package and debian/* is >>> separate from the upstream source. Also, what does it mean to clarify a >>> license? >>> >> >> It used to be GPL-2, and I'm talking about the upstream license. The >> upstream updated it to GPL-3 in 2022, which was actually after Thomas >> last worked on the package. I think maybe I should change the wording >> to "Update license to GPL-3+ following upstream changes"[5] > > - Update license to GPL-3+ following upstream changes > + Clarify license to be GPL-3+ to be consistent with upstream > > doesn't address the issues I raised. > Now added text to clarify the previous license and change date in d/changelog[3]. >>>>>>> - Add Upstream-Contact >>> >>> Thanks for this and for all the other work I didn't comment on. >>> >>> Here are some things you can work on while waiting for a reply from >>> debian-legal: >>> >>> * lintian-explain-tags prefer-uscan-symlink: if you're changing the >>> watch file then this should be addressed >>> >> >> I think this lintian tag is to some extent controversial. See >> bug#1001458[7] where point 2 of the bug submitter makes much sense to >> me, which is I think this lintian tag is marked experimental (X). I >> think if we want to promote the use of "filenamemangle" over "mode=git", >> we may probably consider suppressing this tag in dh-elpa like other >> lintian overrides. Wdyt? > > Overrides need to be documented, and properly documenting your work is > something you still need to work on, and that frequently delays your > reviews, so it's up to you. > It looks like the benefit of this tag is still under debate[4]. Also I think your other comment regarding Lintian tag priorities is a good reference: `prefer-uscan-symlink' is experimental. Therefore I would like to leave it as-is for now. >>> * Finally, have you installed and tested your updated package? >>> >> >> Yes, briefly. Though I'm not a heavy user of HTML templates, I did try >> out the examples/tests in web-mode source and they work once turning on >> web-mode. > > Cool, thanks for confirming. > >>> * Extra/bonus: Which tags from the lintian output are candidates for >>> an override, and why? >>> >> >> Personally I think that most lintian tags suggest good practices but not >> all of them are applicable or suggestible. One example is the >> aforementioned "prefer-uscan-symlink" where the suggestion is local-only >> while the filenamemangle setting can be shared with the team. > > That's a good point. Is there a way to do both at the same time? If > so, lintian should be updated; Actually it looks like this tag is not a good example as it is under debate and people are advocating for its removal[4]. > don't feel obliged to, of course, because human time and motivation is > our most valuable resource. If the majority of people override > "prefer-uscan-symlink" with its current definition, then the tag will > need to be renamed when it is updated for it to go into effect and, in > your own words "deprecate obsolete practices"; this creates a lot of > work for everyone who overrode it. > > Given this, what do you think is the optimal solution for web-mode? > I think in this particular case leaving as-is may be the best option to avoid additional work given the tag is experimental (so not release critical as you suggested later) and when its removal happens we don't need to do more work in the future. >> Another area I can think of is that upstream practice is different >> from lintian's recommendations but works well, like the shard library >> location warnings for native compiled eln which I believe dh-elpa >> overrides, or upstream may have a different form of copyright (like in >> Org in one of the packages I worked on) which is different from the >> recommended format. There must be more cases I haven't thought of. >> On the other hand, I believe lintian should and is also evolving to >> accommodate newer use cases and deprecate obsolete practices. > > Good perspective, and I'm happy to read that you're thinking about these > things. Please use accurate language when speaking of lintian tags, > because error != warning != info != pedantic != experimental, because > errors and warnings are generally release critical. > Ack. I would also like to point out that I have taken the liberty to add myself to the entry of `debian/*' in d/copyright because I believe my commits qualify as intellectual contribution to the packaging work. There are also commits from Thomas Koch, David Bremner, Sean Whitton, and you. Thomas obviously qualifies based on the thread at[5]. I believe David and Sean's commits are just for rebuilding the Emacs addons against dh-elpa for transition, which are not actually work related to the packaging of web-mode so I didn't add them. I think your commits also qualifies, but since you are active I'll leave it up to you. Thanks again for your review, and PTAL. > Regards, > Nicholas [1] https://salsa.debian.org/emacsen-team/web-mode/-/commit/cfe8b181b09ce95a2506f3e6047e3835df2b0b57 [2] https://salsa.debian.org/emacsen-team/web-mode/-/commit/4febc291170bcaa7cc75a0ab73766564d29ba828 [3] https://salsa.debian.org/emacsen-team/web-mode/-/commit/4c68a581a5413092c6053ffb6cd0a397e6b3fa28 [4] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005200 [5] https://lists.debian.org/debian-legal/2024/04/msg00001.html -- Regards, Xiyue Deng
signature.asc
Description: PGP signature
