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?

>> 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.  Also, as stated before, I won't touch anything
dgit-related.

>> 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.

>>>>>>      - 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.

>>   * 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; 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?

> 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.

Regards,
Nicholas

Attachment: signature.asc
Description: PGP signature

Reply via email to