nodejs-packaging

#13 Refactor/rewrite nodejs.req
Closed by tomh. Opened by jstanek.
jstanek/nodejs-packaging pr/refactor-nodejs.req  into  master

Download 13.patch

This is a refactor in spirit similar to nodejs-packaging#12. Since the Requires generator is more complex, the refactor is also bigger.

As before, I will gladly explain and amend any confusing piece of code pointed to me ☺.

Goals

  1. Compatibility with both Python 2 (because EPEL) and Python 3 (because future).

  2. Backward compatibility: The previous behavior were kept as much as possible (down to typos in warning message), and I would treat any found diversion as a bug at this point. The test suite passes without any changes necessary.

  3. Readability and separation of concerns: The ad-hoc parsing routines were replaced with smaller, dedicated, and documented methods/functions.

  4. Better test coverage: Functions that implement the semver parsing (and few extras) have examples specified in the form of doctests, and the test suite now runs them to make sure they behave as expected.

Isn't this going to cause warnings because it's using unversioned python? That's why I changed it to python3 a while back....

Also nodejs.prov is still python3 ever after your rewrite, do they not both need to support python2 to be of any use to EPEL?

Does EPEL really not have python3 by the way?

Does EPEL really not have python3 by the way?

It does, of course (python 3.6 nowadays), and I really should stop using EPEL in the meaning "any enterprise distribution based on old Fedora". My bad.

The concern here is to allow running this on i.e. CentOS 7 (without EPEL packages; sigh) ideally without code changes. Both my nodejs.req and nodejs.prov are written so that both python2 and python3 works.

Isn't this going to cause warnings because it's using unversioned python? That's why I changed it to python3 a while back....

It emits an error, actually; thanks for pointing it out. I will change it to python3, replacing it with python2 on older distributions is not a problem if the code can be kept the same.

1 new commit added

  • fixup! Refactor nodejs.req

Ping! Any other issues or comments? Is there anything I can do to get this merged?

Sorry I didn't realise you had actually made the change. I'll try and have a look later.

Before I do a new release are you likely to be making any more changes? Only if you are I'll probably delay making an actual release until you're done.

Before I do a new release are you likely to be making any more changes? Only if you are I'll probably delay making an actual release until you're done.

Yes. I have some slight changes to the nodejs.req behavior planned (namely bundled/un-bundled module detection) that I did not want to intersperse with the refactor. Thanks for the consideration.

Pull-Request has been closed by tomh
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.