Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Nicholas D Steeves writes: > [[PGP Signed Part:Undecided]] > Xiyue Deng writes: > >> Nicholas D Steeves writes: >>> Xiyue Deng writes: Nicholas D Steeves writes: >> >>> 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. > > Putting yourself in Uploaders means you're going to take a special (and > regular) interest in a package in the long term. Is that really what > you intend? > I will regularly monitor the bug tracker and fix any issues regarding building and tests. I hope this is sufficient to become an uploader. >>> Meanwhile, compression type isn't the problem: >>> >>> gbp:info: Tarballs 'scala-mode-el_1.1.0+git20221025.5d7cf21.orig.tar.gz' >>> not found at '/home/sten/devel/tarballs/' >>> gbp:error: v1.1.0+git20221025.5d7cf21 is not a valid treeish >>> >> >> This is because I didn't push the tag to the repo yet. Now this is >> done. > > Thanks. When pushing upstream history, one needs to also push the > upstream tag. > >>> and at the same time, the proposed switch to xz is also arguably a >>> regression, because the actual upstream release tarballs of scala-mode >>> are currently being used: >>> >>> $ apt source scala-mode-el >>> $ md5sum scala-mode-el_1.1.0.orig.tar.gz >>> 615b1f38ed083137fee99fa83fe452a1 scala-mode-el_1.1.0.orig.tar.gz >>> # Download release tarball from github manually, or use uscan >>> $ md5sum ~/Downloads/emacs-scala-mode-1.1.0.tar.gz >>> 615b1f38ed083137fee99fa83fe452a1 >>> /home/sten/Downloads/emacs-scala-mode-1.1.0.tar.gz >>> >>> This indicates that the human maintainer doesn't use "git deborig". >>> Being able to reproduce the imports of official upstream tarballs is >>> important to a number of developers, and some workflows depend on this >>> assumption, so please don't break it. >>> >> >> 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. >> >> 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. 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, >> which I believe is incompatible with "gbp import-orig --uscan" approach >> which directly import the release tarball without the git history. > > What does "IIUC" mean? "If I understand correctly" > This repository doesn't use dgit workflow. Gbp can optionally merge > git history. Ack. I also converted my patch using Gbp Pq and updated the forward info[1]. > > If upstream provides release tarballs in gz rather than xz, and the > Debian Maintainer/Uploader uses those, then the repository needs to be > configured for gz and not xz. Note that d/watch previously specified > gz. Changing that configuration is working against the > Maintainer/Uploader. Given this, it is more correct to configure gbp to > use gz until upstream switches to xz. I will buy your argument in this package and hence revert that change in [2]. Though I still believe preparing a release from a git tag has its value in general, especially in post-Jia-Tan world. I believe tag2upload is another step towards that goal. > "Git deborig" should also be invoked to use gz. > See above. >> 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. > > What do you mean? Just use uscan, or origtargz. Or do you mean > accommodating the case where you think moving from a stable release to a > snapshot is justified? Tell upstream that Debian highly values stable > releases, and convince them to tag one. Problem solved. P.S. I read a > message you wrote to an upstream about this, and that github issue > didn't look like it would convince anyone of the value of tagged > releases...please do better in the future. > Well, I think part of the problem is that the upstream is essentially MIA as the last commit was 6 months ago, which means a new tagged release is not likely to happen any time soon. Preparing the latest snapshot would be the next best thing we can do IMHO. Anyway, PTAL with my recent commits that fixes some of the issues you mentioned. > Nicholas > > [[End of PGP Signed Part]] [1] https://salsa.debian.org/emacsen-team/scala-mode-el/-/commit/ed8cf56a432351c743a139173c5f121f1f73db6c [2] https://salsa.debian.org/emacsen-team/scala-mode-el/-/commit/857b767743df6548a821f070711dcc9e1b8df739 -- Xiyue Deng signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Hi Nicholas, Nicholas D Steeves writes: > [[PGP Signed Part:Undecided]] > Xiyue Deng writes: > >> With the release of the new policy version, I have done some more clean >> up to the package and update team repo and mentors. PTAL. TIA! > > Please stop making copyright assertions for other people. We've > discussed this before, and it's the actual copyright situation is what > counts and not lintian's experimental "update-debian-copyright" tag. > That tag is to remind contributors in the d/copyright file to check > whether they've done any work that meets the > > https://en.wikipedia.org/wiki/Threshold_of_originality > Acknowledged. I was trying to revert that change and saw you already did it in [1]. Thanks. Also thanks for the link. I was trying to find more explanations regarding Copyright concepts. This also explains why some DDs consider the contents under `debian/' not copyrightable because the lack of originality. Anyway, in normal case, I would think adding oneself to the copyright list of `File: debian/*' when doing any changes is acceptable as the changes reflects the intention of oneself to improve the package, even only incrementally. > You also need to explain what "Drop old version on emacs in recommends." > means, and *why* you made this change, because it's not self-evident. Clarified in [2]. > > Cheers, > Nicholas > > [[End of PGP Signed Part]] [1] https://salsa.debian.org/emacsen-team/scala-mode-el/-/commit/5d670b9cc33f22d34d3003c6606f785f05644d36 [2] https://salsa.debian.org/emacsen-team/scala-mode-el/-/commit/0f7d996aaf8efc4ffc36f63351045fc8e498bb68 -- Xiyue Deng
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Xiyue Deng writes: > Nicholas D Steeves writes: >> Xiyue Deng writes: >>> Nicholas D Steeves writes: > >> 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. Putting yourself in Uploaders means you're going to take a special (and regular) interest in a package in the long term. Is that really what you intend? >> Meanwhile, compression type isn't the problem: >> >> gbp:info: Tarballs 'scala-mode-el_1.1.0+git20221025.5d7cf21.orig.tar.gz' >> not found at '/home/sten/devel/tarballs/' >> gbp:error: v1.1.0+git20221025.5d7cf21 is not a valid treeish >> > > This is because I didn't push the tag to the repo yet. Now this is > done. Thanks. When pushing upstream history, one needs to also push the upstream tag. >> and at the same time, the proposed switch to xz is also arguably a >> regression, because the actual upstream release tarballs of scala-mode >> are currently being used: >> >> $ apt source scala-mode-el >> $ md5sum scala-mode-el_1.1.0.orig.tar.gz >> 615b1f38ed083137fee99fa83fe452a1 scala-mode-el_1.1.0.orig.tar.gz >> # Download release tarball from github manually, or use uscan >> $ md5sum ~/Downloads/emacs-scala-mode-1.1.0.tar.gz >> 615b1f38ed083137fee99fa83fe452a1 >> /home/sten/Downloads/emacs-scala-mode-1.1.0.tar.gz >> >> This indicates that the human maintainer doesn't use "git deborig". >> Being able to reproduce the imports of official upstream tarballs is >> important to a number of developers, and some workflows depend on this >> assumption, so please don't break it. >> > > 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. > > 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. 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, > which I believe is incompatible with "gbp import-orig --uscan" approach > which directly import the release tarball without the git history. What does "IIUC" mean? This repository doesn't use dgit workflow. Gbp can optionally merge git history. If upstream provides release tarballs in gz rather than xz, and the Debian Maintainer/Uploader uses those, then the repository needs to be configured for gz and not xz. Note that d/watch previously specified gz. Changing that configuration is working against the Maintainer/Uploader. Given this, it is more correct to configure gbp to use gz until upstream switches to xz. "Git deborig" should also be invoked to use gz. > 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. What do you mean? Just use uscan, or origtargz. Or do you mean accommodating the case where you think moving from a stable release to a snapshot is justified? Tell upstream that Debian highly values stable releases, and convince them to tag one. Problem solved. P.S. I read a message you wrote to an upstream about this, and that github issue didn't look like it would convince anyone of the value of tagged releases...please do better in the future. Nicholas signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Xiyue Deng writes: > With the release of the new policy version, I have done some more clean > up to the package and update team repo and mentors. PTAL. TIA! Please stop making copyright assertions for other people. We've discussed this before, and it's the actual copyright situation is what counts and not lintian's experimental "update-debian-copyright" tag. That tag is to remind contributors in the d/copyright file to check whether they've done any work that meets the https://en.wikipedia.org/wiki/Threshold_of_originality You also need to explain what "Drop old version on emacs in recommends." means, and *why* you made this change, because it's not self-evident. Cheers, Nicholas signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Xiyue Deng writes: > > > It looks like the bug was archived so the previous mail didn't reach > BTS. So unarchived, reopened, and retitle the bug with newer snapshot, > and also resending the following from previous message. > Friendly ping for comments. -- Xiyue Deng signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Control: retitle -1 RFS: scala-mode-el/1:1.1.0+git20240113.4c6d636-1 [RC] [Team] -- Emacs major mode for editing scala source code Xiyue Deng writes: > Hi, > > Xiyue Deng writes: > >> Nicholas D Steeves writes: >> >>> Hi, >>> >>> Paul Wise writes: >>> On Mon, 2023-12-04 at 02:28 -0800, Xiyue Deng wrote: > I think dh_auto_clean is the right place, because the build failure is > because that the clean target requires the existence of > scala-mode-pkg.el, which is generated by Cask. As we don't have Cask, > we need to provide this before dh_auto_clean runs. >>> >>> The generation of this metadata, and file, is one of dh_elpa's primary >>> functions. See the section of the dh_elpa man page that discusses >>> DEB_VERSION_UPSTREAM. >> >> Ah I see what you mean: as long as dh_elpa can detect the version >> correctly we don't need to provide an actual *-pkg.el file and can just >> let dh_elpa generate it, which also avoids the potential policy >> violation Paul pointed out. >> >> So now I make override_dh_auto_clean to call dh_elpa to generate this >> file for use. However, as the generated file is "buried" pretty deep in >> the generated directory, the command becomes pretty long, but it works. >> Admittedly that requiring a generated file during clean is pretty exotic >> and I haven't encountered it elsewhere (yet) which is good. >> >> New handling strategy pushed to team repo. PTAL. >> >>> >>> Read Policy §4.9 closely; Cask cannot be used. >>> >>> Grep the buildlog for "dh_" and if you'd like to see a more >>> comprehensive list of applicable entry points in the sequence, try >>> >>> $ dh binary-indep --no-act >>> >>> It's also worth reading the dh man page. >> >> Ack. >> >>> I think it is against ftp-master rules to have generated files present that can't be built using only tools from Debian main. >>> >>> Yes, and thank you for bringing this up. >>> So I think you would need to package Cask first? >>> >>> Cask is not relevant nor needed, and dh_elpa is used. Whenever this >>> topic comes up on IRC, new contributors are briefed and are additionally >>> referred to the RFP for Cask, where the unsuitability of this type of >>> tool for Debian packaging is discussed (#837922). It's also worth >>> noting that dh_elpa was written by people by current and/or past members >>> of the Debian TC. >>> >>> Xiyue Deng writes: >>> Cask and similar tools like Eask and Eldev are tools that automatically install dependencies of an Emacs addon package, which doesn't use and circumvents the system package management. I think the Emacsen team chooses not to package those tools and prefers using dh-elpa for the job, and may override build target to avoid using those tools. >>> >>> If you're familiar with the concept of "hats", then when you're working >>> on debian/* you need to put on your Debian packaging hat, and when >>> you're working on !(debian/*) you use your upstream >>> development/debugging/packaging hat. These tools are not relevant to >>> Debian packaging and referring to them is not useful for describing >>> packaging problems or decisions; there will always be a more direct and >>> useful description. >> >> I think those external tools are not completely irrelevant but just in >> the sense that we do it the Debian way. Usually they provide two types >> of functions: 1) automatic dependency management, and 2) automatic file >> generation required for testing and distribution through elpa. In >> Debian, the former is handled by apt, and the latter by dh_elpa, and we >> take effort to make sure they behave the same. >> >>> >>> Cheers, >>> Nicholas >>> It looks like the bug was archived so the previous mail didn't reach BTS. So unarchived, reopened, and retitle the bug with newer snapshot, and also resending the following from previous message. > > With the release of the new policy version, I have done some more clean > up to the package and update team repo and mentors. PTAL. TIA! -- Xiyue Deng signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Nicholas D Steeves writes: > Hi, > > Paul Wise writes: > >> On Mon, 2023-12-04 at 02:28 -0800, Xiyue Deng wrote: >> >>> I think dh_auto_clean is the right place, because the build failure is >>> because that the clean target requires the existence of >>> scala-mode-pkg.el, which is generated by Cask. As we don't have Cask, >>> we need to provide this before dh_auto_clean runs. > > The generation of this metadata, and file, is one of dh_elpa's primary > functions. See the section of the dh_elpa man page that discusses > DEB_VERSION_UPSTREAM. Ah I see what you mean: as long as dh_elpa can detect the version correctly we don't need to provide an actual *-pkg.el file and can just let dh_elpa generate it, which also avoids the potential policy violation Paul pointed out. So now I make override_dh_auto_clean to call dh_elpa to generate this file for use. However, as the generated file is "buried" pretty deep in the generated directory, the command becomes pretty long, but it works. Admittedly that requiring a generated file during clean is pretty exotic and I haven't encountered it elsewhere (yet) which is good. New handling strategy pushed to team repo. PTAL. > > Read Policy §4.9 closely; Cask cannot be used. > > Grep the buildlog for "dh_" and if you'd like to see a more > comprehensive list of applicable entry points in the sequence, try > > $ dh binary-indep --no-act > > It's also worth reading the dh man page. Ack. > >> I think it is against ftp-master rules to have generated files >> present that can't be built using only tools from Debian main. > > Yes, and thank you for bringing this up. > >> So I think you would need to package Cask first? > > Cask is not relevant nor needed, and dh_elpa is used. Whenever this > topic comes up on IRC, new contributors are briefed and are additionally > referred to the RFP for Cask, where the unsuitability of this type of > tool for Debian packaging is discussed (#837922). It's also worth > noting that dh_elpa was written by people by current and/or past members > of the Debian TC. > > Xiyue Deng writes: > >> Cask and similar tools like Eask and Eldev are tools that automatically >> install dependencies of an Emacs addon package, which doesn't use and >> circumvents the system package management. I think the Emacsen team >> chooses not to package those tools and prefers using dh-elpa for the >> job, and may override build target to avoid using those tools. > > If you're familiar with the concept of "hats", then when you're working > on debian/* you need to put on your Debian packaging hat, and when > you're working on !(debian/*) you use your upstream > development/debugging/packaging hat. These tools are not relevant to > Debian packaging and referring to them is not useful for describing > packaging problems or decisions; there will always be a more direct and > useful description. I think those external tools are not completely irrelevant but just in the sense that we do it the Debian way. Usually they provide two types of functions: 1) automatic dependency management, and 2) automatic file generation required for testing and distribution through elpa. In Debian, the former is handled by apt, and the latter by dh_elpa, and we take effort to make sure they behave the same. > > Cheers, > Nicholas > -- Xiyue Deng signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Hi, Paul Wise writes: > On Mon, 2023-12-04 at 02:28 -0800, Xiyue Deng wrote: > >> I think dh_auto_clean is the right place, because the build failure is >> because that the clean target requires the existence of >> scala-mode-pkg.el, which is generated by Cask. As we don't have Cask, >> we need to provide this before dh_auto_clean runs. The generation of this metadata, and file, is one of dh_elpa's primary functions. See the section of the dh_elpa man page that discusses DEB_VERSION_UPSTREAM. Read Policy §4.9 closely; Cask cannot be used. Grep the buildlog for "dh_" and if you'd like to see a more comprehensive list of applicable entry points in the sequence, try $ dh binary-indep --no-act It's also worth reading the dh man page. > I think it is against ftp-master rules to have generated files > present that can't be built using only tools from Debian main. Yes, and thank you for bringing this up. > So I think you would need to package Cask first? Cask is not relevant nor needed, and dh_elpa is used. Whenever this topic comes up on IRC, new contributors are briefed and are additionally referred to the RFP for Cask, where the unsuitability of this type of tool for Debian packaging is discussed (#837922). It's also worth noting that dh_elpa was written by people by current and/or past members of the Debian TC. Xiyue Deng writes: > Cask and similar tools like Eask and Eldev are tools that automatically > install dependencies of an Emacs addon package, which doesn't use and > circumvents the system package management. I think the Emacsen team > chooses not to package those tools and prefers using dh-elpa for the > job, and may override build target to avoid using those tools. If you're familiar with the concept of "hats", then when you're working on debian/* you need to put on your Debian packaging hat, and when you're working on !(debian/*) you use your upstream development/debugging/packaging hat. These tools are not relevant to Debian packaging and referring to them is not useful for describing packaging problems or decisions; there will always be a more direct and useful description. Cheers, Nicholas signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Hi Paul, Paul Wise writes: > On Mon, 2023-12-04 at 02:28 -0800, Xiyue Deng wrote: > >> I think dh_auto_clean is the right place, because the build failure is >> because that the clean target requires the existence of >> scala-mode-pkg.el, which is generated by Cask. As we don't have Cask, >> we need to provide this before dh_auto_clean runs. > > I think it is against ftp-master rules to have generated files > present that can't be built using only tools from Debian main. > > So I think you would need to package Cask first? Cask and similar tools like Eask and Eldev are tools that automatically install dependencies of an Emacs addon package, which doesn't use and circumvents the system package management. I think the Emacsen team chooses not to package those tools and prefers using dh-elpa for the job, and may override build target to avoid using those tools. See also [1] and [2] for some previous discussions. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=837922#15 [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=875722#16 -- Xiyue Deng signature.asc Description: PGP signature
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
On Mon, 2023-12-04 at 02:28 -0800, Xiyue Deng wrote: > I think dh_auto_clean is the right place, because the build failure is > because that the clean target requires the existence of > scala-mode-pkg.el, which is generated by Cask. As we don't have Cask, > we need to provide this before dh_auto_clean runs. I think it is against ftp-master rules to have generated files present that can't be built using only tools from Debian main. So I think you would need to package Cask first? -- bye, pabs https://wiki.debian.org/PaulWise signature.asc Description: This is a digitally signed message part
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Nicholas D Steeves writes: > Xiyue Deng writes: > >> Nicholas D Steeves writes: >>> Xiyue Deng writes: Nicholas D Steeves 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. Indeed, updated the changelog with this[2]. > >> 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. I think dh_auto_clean is the right place, because the build failure is because that the clean target requires the existence of scala-mode-pkg.el, which is generated by Cask. As we don't have Cask, we need to provide this before dh_auto_clean runs. > > Please use "cp -a" so timestamps between builds will be reproducible. Done. > What is the rationale for CURDIR? From what I can tell it isn't needed > and should be dropped. I can't find a packaging suggestion on $(CURDIR), but it looks like it also works without it, so I dropped it[3]. > >>> 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. Somehow I didn't include this patch I submitted upstream in my last changes. This is done now[1]. > >> 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 >>> i
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Xiyue Deng writes: > Nicholas D Steeves writes: >> Xiyue Deng writes: >>> Nicholas D Steeves 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 mean
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Nicholas D Steeves writes: > Xiyue Deng writes: > >> Nicholas D Steeves writes: >> >> >>> Have you asked upstream to tag a release? >>> >> >> Not before your review but done by now at [1] > > Thank you. You may have heard that Debian is a distribution that > privileges the stable release model... When the human maintainer of a > Debian package tracks stable releases, why is importing a snapshot > justified? > 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. 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. > 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. * Override clean rules in d/rules to fix building. (Closes: #1052917) >>> >>> I believe you already know that >>> >>> override_dh_auto_clean: >>>/bin/true >>> >>> is an incorrect approach. >>> >> >> Indeed it was not ideal. Upstream depends on Cask to generated the >> scala-mode-pkg.el file that is used in the clean target to get the name >> of the generated tarball, and indeed using this lazy approach is >> incorrect. I've now included the generated pkg file through a patch to >> make this work in [2]. > > Consistency is essential between an explanation (in a comment or > changelog) and the work that was done. > > Statically defining package metadata is fine, but in this case you can't > claim that you're generating the pkg.el file. Oh yes it's generated using Cask following upstream practice. I just include it as a patch as we don't use Cask for Debian packaging. > Either make the changelog and patch description consistent with what > is actually happening, or change the implementation so that something > is actually generated (there are multiple approaches here). I think I > tend to use makefile substvars for this. Personally I prefer not to patch upstream source if not necessary, otherwise we end up carrying the patch for the foreseeable future. Though arguably in this case the scala-mode-pkg.el file needs to be generated/patched which requires I use Cask locally, so it's more or less the same effort. 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]. > 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]. > 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 Anyway, I have just made a pull request to update this upstream[4] so hopefully the versioning will be sane. * 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. * Add more metadata in d/upstream/metadata. >>> >>> https://github.com/hvesalai/emacs-scala-mode/commits/master >>> >>> is a git history log, not a changelog nor release notes. >>> >> >> I thought the git history log may be considered an alternative form of a >> Changelog. Looks like I was wro
Bug#1055431: RFS: scala-mode-el/1:1.1.0+git20221025.5d7cf21-1 [RC] [Team] -- Emacs major mode for editing scala source code
Xiyue Deng writes: > Nicholas D Steeves writes: > > >> Have you asked upstream to tag a release? >> > > Not before your review but done by now at [1] Thank you. You may have heard that Debian is a distribution that privileges the stable release model... When the human maintainer of a Debian package tracks stable releases, why is importing a snapshot justified? Also, do you use this package? >>>* Override clean rules in d/rules to fix building. (Closes: >>>#1052917) >> >> I believe you already know that >> >> override_dh_auto_clean: >>/bin/true >> >> is an incorrect approach. >> > > Indeed it was not ideal. Upstream depends on Cask to generated the > scala-mode-pkg.el file that is used in the clean target to get the name > of the generated tarball, and indeed using this lazy approach is > incorrect. I've now included the generated pkg file through a patch to > make this work in [2]. Consistency is essential between an explanation (in a comment or changelog) and the work that was done. Statically defining package metadata is fine, but in this case you can't claim that you're generating the pkg.el file. Either make the changelog and patch description consistent with what is actually happening, or change the implementation so that something is actually generated (there are multiple approaches here). I think I tend to use makefile substvars for this. 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"? 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)? >>>* 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. >>>* Add more metadata in d/upstream/metadata. >> >> https://github.com/hvesalai/emacs-scala-mode/commits/master >> >> is a git history log, not a changelog nor release notes. >> > > I thought the git history log may be considered an alternative form of a > Changelog. Looks like I was wrong except for projects that requires the > same format across changelog/git history/release notes. I've dropped > that line in [3]. Thank you. Re: "projects that requires the same format across changelog/git history/release notes": Changelogs, NEWS files, and release notes are three (or arguably two) distinct types of documentation that are also distinct from VCS history. This isn't a superficial formatting or style thing, because they have different audiences and purposes. I think that the kind of changelog that you're probably thinking of it when upstream takes git's shortlog history, puts it in a file, and edits it so that it makes sense. >>>* Update year and Upstream-Contact and add myself in d/copyright. >> >> Why did you add yourself? >> https://en.wikipedia.org/wiki/Threshold_of_originality >> >> I'm happy to support your claim, but you'll need to work for it in more >> than a "sweat of the brow"/mechanical sense. >> > > To be honest, the only reason I did this is to suppress the > "update-debian-copyright" lintian warning which is actually > experimental. I believe what I did was in the same nature as Sławomir > did in 2020 though admittedly not to the same extent, so I've reverted > this part in [4]. Cool. Yeah, lintian has these tags: error, warning, info, pedantic, experimental. Which ones do you think are suggestions, and which one[s] require a mandatory fix? Note that suggestions are occasionally highly opinionated and conflict with team policy. As ever, it's not sufficient to simply react to lintian: ie "lintian made me do it!". >>>* Use xz compression in d/gbp.conf. >> >> Why is this useful when it has been the default since gbp 0.9.15? >> > > I'm pretty sure that if I don't add this "git deborig" will create the > tarball using
