Xiyue Deng <manphiz@gmail.com> writes: > Nicholas D Steeves <sten@debian.org> writes: >> Xiyue Deng <manphiz@gmail.com> writes: >>> Nicholas D Steeves <sten@debian.org> writes: >>> > > There are about 3 years of newer commits since 1.1.0, and one of the > major changes is that it adds support of scala 3 syntax which is not yet > in a tagged release and would be good to have. Ok, you've convinced me :) Convey this information in your changelog (that's what it's for), because the human maintainer (and any interested users) of this package deserves to know why you made this change. > Also seeing the last commit is from the end of last year, I sense that > upstream may becoming a bit dormant for the time being, which is why I > prefer to package the latest snapshot instead of waiting for a stable > release. This can also mean that we run the risk of becoming defacto upstream if they quit at this point, but that said, I agree it's a good cut point as well as the right thing to do. >> Also, do you use this package? >> > > Not on a regular basis, but I do use it a bit once in a while as I try > to learn a bit of new programming languages every few months. Thanks for confirming! [snip] > And then I just realized: why not just host the scala-mode-pkg.el file > and substitute the version so that we don't need to update it manually > on each update? This is now implemented at [1]. Substvars make sense ;) Also, neat use of a makefile target called from within the dh sequence. Are you sure dh_auto_clean is the right place for this override? Skim its man page, as well as the one for dh_clean before replying. Also, whenever one overrides something in rules, one needs to document this in the changelog. Please use "cp -a" so timestamps between builds will be reproducible. What is the rationale for CURDIR? From what I can tell it isn't needed and should be dropped. >> Do you see what will happen when the package is updated to >> 1.1.1 or newer? Also, why did you choose to set the version to "0.23" >> rather than "1.1.0"? > > Well I didn't choose it (if I did I'd use 1.1.0 :) This is the version > specified in scala-mode.el[2]. I like your choice, and so what if upstream has that! Is it correct? >> Did you verify that elpa package version is consistent with the >> upstream version of the Debian package in bin:elpa-scala-mode that is >> consumed by users (the binary package)? >> > > I tried install it from stable.melpa.org and yeah it's being > installed as scala-mode-0.23 even if it's registered as version 1.1.0 > there[3]. So it's consistent in a sense :P Oh my! Sorry for the convoluted sentence I wrote, and I'm impressed that you were able to make sense of it. Yes, stable.melpa.org publishes a scala-mode version 1.1.0 elpa package, which contains a scala-mode.el file with "Package-Version: 0.23", and it also contains a scala-mode-pkg.el file that overrides the Package-Version to `1.1.0`. It is because of this pkg.el file that elpa/scala-mode-1.1.0 subdir. Meanwhile our elpa-scala-mode 1.1.0-* all declare 0.23, and install to a scala-mode-0.23 subdir. Thank you for your kind optimism, that's very gracious. Your work reveals that I missed this issue when reviewing 1.1.0-1, which I appreciate having pointed out. Lets fix it in the upload you've proposed. > Anyway, I have just made a pull request to update this upstream[4] so > hopefully the versioning will be sane. Thank you, and hopefully they'll agree. >>>>> * Modernize d/watch using special substitute strings. >>>> >>>> Ok, but why? >>>> >>> >>> I believe this provides a more robust way of detecting tags and should >>> be an encouraged practices. From my own experience, when I find a >>> d/watch file that doesn't work I may search for other packages to learn >>> from existing practices, and some may not work well as different >>> upstream may follow different conventions. The substitute strings use a >>> more robust and tested regexp that works most of the time, and promoting >>> its use may save people's time instead of working on an ad-hoc regexp. >> >> Sounds good! This is the kind of rationale that should be in the >> changelog, so please add it there :) From now on, read your changelog >> and patche desriptions, and imagine I'm asking you "ok, but why" for >> each point. Yes, rarely something is self-evident and/or an >> implementation detail, but most of the time you should say a few words >> explaining "why"--particularly when you want to find a sponsor quickly. >> My expectation is that you get better at this with each review, and that >> you will apply everything you learned to all pending sponsorship >> requests in addition to future ones. >> > > Ack, and good suggestion! I have slightly extended the entry with the > rationale[5]. PTAL. Thanks. LTGM. P.S. If you wanted to "make a mark" on many packages, a possible project could be to extend lintian to detect when @ANY_VERSION@@ARCHIVE_EXT@ can be used, but isn't yet being used. >>>>> * Use xz compression in d/gbp.conf. [snip] > I meant to say "gbp buildpackage" which uses this field to specify the > compression type. Sorry for the confusion. No worries! [snip] > In this specific case as I'm targeting a snapshot so there is no > upstream tarball, so using xz provides the benefits of a smaller source > tarball that saves space for the Debian source repository. I agree this makes sense, particularly when using a snapshot. Personally I'm fine with the unreproducible orig.tar checksum when packing a git treeish. I seem to remember that Sławomir Wójcik uploaded the official upstream-released orig.tar.gz to his RFS, and I'd prefer not to break his workflow, since he's the only human maintainer of this package. If you'd like to add yourself to Uploaders then I'll accept the switch to xz as well as other maintainer-level decisions you'd like to make. After all, Wójcik hasn't worked on this package in almost three years. > For the aspect of the consistency with upstream release tarball, I'm > trying to understand how to make this work. I believe "gbp import-orig > --uscan" should grab the upstream release tarball which follows your > suggestion. Yes, that command will download the official release orig.tarball, unpack it to the upstream branch, then merge the upstream[via-tarball]/tag to the Debian branch; this is only used when importing a new release. For a -2 revision, on uses uscan --download-current-version. > However IIUC the repository is configure using the > dgit-maint-merge workflow that uses upstream branch to target upstream > repo and hence uses the tagged version to generate upstream tarball, Close! To be fair, there are many workflows. If I remember correct the current state of this repo is a manual tag-merging workflow without dgit-maint-merge. > which I believe is incompatible with "gbp import-orig --uscan" approach > which directly import the release tarball without the git history. Yes, that's correct for that command. Gbp can also import from tags, and it has some kind of hybrid tarball + git history mode too, but I can't remember ever using it. > I wonder whether there is a way for "git deborig" or "gbp > buildpackage" to grab upstream tarball automatically? I'm guessing > not, so someone has to do it manually, but please let me know if there > is a way. > Git deborig uses git-archive, so the orig.tarball won't be reproducible. I've probably missed a few solutions, but the ones I know of are: * Gbp's optional pristine-tar support keeps deltas for every tarball on a git branch, and pristine-tar is almost always reproducible--except in extremely rare corner cases. * Origtargz uses a variety of methods (including checking for pristine-tar and using it if available), but origtargz always puts the tarball in ../ rather than in the configured orig.tarball dir; this is fixable, of course, but no one has done it yet. * Then there's uscan. A common, and more old-school workflow is to always call uscan. If the required orig.tar is absent, it will download a copy. Gbp's "git-prebuild" config key with uscan should do the trick. And of course there are always shell aliases as well as custom wrapper scripts. I wrote a wrapper script that checks for all the things that I tend to forget. It's nice to be able to type "build" or "release" and have it do the right thing, even for an irregular case that one hasn't seen in a year and a half. Best, Nicholas
Attachment:
signature.asc
Description: PGP signature