LeoIannacone / npm2deb

tool to help debianize Node.js modules
GNU General Public License v3.0
46 stars 34 forks source link

Remove node-<module> directory if uupdate is successful #84

Closed shanavas786 closed 4 years ago

shanavas786 commented 7 years ago

npm2deb creates node-module and node-module-version directories which is a bit confusing for new contributers.

wookey commented 4 years ago

Also reported in the Debian BTS at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=961873

wookey commented 4 years ago

There is now a patch in the above bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=961873;filename=npm2deb-0.3.0-fix-961873-2.patch;msg=22

Which is actually pretty trivial, and fundamentally amounts to:

--- npm2deb-0.3.0.orig/npm2deb/__init__.py
+++ npm2deb-0.3.0/npm2deb/__init__.py
@@ -100,6 +100,9 @@ class Npm2Deb(object):

             new_dir = '%s-%s' % (self.debian_name, self.upstream_version)
             utils.change_dir('../%s' % new_dir)
+            # copy over non-duplicated changelog
+            _os.rename('../%s/debian/changelog' % self.debian_name, 'debian/ch
angelog')
+            _rmtree('../%s' % self.debian_name) 
             self.run_buildpackage()
             self.edit_changelog()

and

--- npm2deb-0.3.0.orig/npm2deb/templates.py
+++ npm2deb-0.3.0/npm2deb/templates.py
@@ -1,4 +1,4 @@
-CHANGELOG = """%(debian_name)s (%(version)s-1) UNRELEASED; urgency=low
+CHANGELOG = """%(debian_name)s (%(version)s-1) UNRELEASED; urgency=medium

   * Initial release (Closes: #nnnn)
pravi commented 4 years ago

@wookey would you mind creating a pull request here, so its easy to review (at least one committer should review a PR)?

wookey commented 4 years ago

Sorry, I don't know how to do pull requests. I posted the patch here as well as in the debian bug, in both 'obvious' (readable) and debdiff (just apply it) forms. I've fixed up the formatting here so it's easier to read. Is it really too hard to review a 3-line patch? I've been using it successfully here for a few packages already. It works.

pravi commented 4 years ago

@wookey it is about the general process requirement we adopted, not specific to this patch. We prefer at least one other person review all changes. I can create a pull request for you from the patch, which then @shanavas786 can review.

wookey commented 4 years ago

Sure. Upstream has their processes, as they should. I supplied a fix to the Debian package and it's generally best if the package maintainer translates that into whatever process upstream have (as you are doing, thanks). I just posted here as well because you pointed out there was an upstream issue on the same subject. So sorry if I didn't follow the upstream process exactly - it's not something I know (or really want to know - I deal with hundreds of upstreams!) anything about. (And yes I should probably learn this newfagled proprietary github 'Pull Request' thing sometime but I try to have as little to do with proprietary software as possible, and I don't need it outside github).

I'm surprised npm2deb isn't a native package actually - it seems very debian-specific so I wonder what the point of a separate upstream is?

Anway, thanks for dealing with this promptly - I look forward to the next release :-)

pravi commented 4 years ago

@wookey I only wanted to check with you if you are comfortable with sending a pull request directly. As I said earlier, I'm fine doing it for you. As you noted, most upstreams or even many teams in Debian prefers the pull request work flow (in gitlab, it is called merge request). It is nothing but a fork/copy the project, make your changes, publish your changed repo, then ask them to pull from your repo. I find it easier than the patch workflow. I think npm2deb started here is just a historic thing, it was started much before we had salsa. But I agree, moving it to salsa as a native package would be much better. @LeoIannacone thoughts?