The build is disabled, so I cannot review the package.

Hillwood Yang (hillwood) - over 6 years ago

author source maintainer

This Building is enable. Please review.

Now at the moment we do not have any npm packages included in the distro, as they were all previously removed after we stopped building npm separately to nodejs.

Apparently the main reason for these two packages (nodejs-underscore, nodejs-emojione) is to build ibus.

@AdamMajer @MargueriteSu, do you think it makes sense to include npm packages separately like in this example or to bundle them because of potential version mismatches with other packages?

Anyway, a little nit (line 20):

%if 0%{?suse_version} >= 1315 || 0%{?suse_version} <= 1316

Is this conditional not always true?

And for line 33, can't those two conditionals be replaced by a single != operator?

@MargueriteSu

@qantas94heavy

Hi, in general, both packages are okay (nodejs-underscore, nodejs-emojione).

1. they're not packages that required lots of annoying dependencies. they are self-satisfied.

2. in my opinion, to have or not to have them in factory, matters little with the future of nodejs packaging. because @hillwood manually copy the files in the .tgz tarball, so no matter how packaging changes, they'll stay the same.

3. to me, they so much look like just two packages accidentally have the prefix "nodejs-" :-)

and they're packages that made ibus full functional. so please let them in.

@hillwood

+%if 0%{?suse_version} < 1315 || 0%{?suse_version} > 1316
+BuildRequires:  nodejs-packaging
+%endif

did you want

+%if 0%{?suse_version} == 1315 

here?

Hillwood Yang (hillwood) - over 6 years ago

author source maintainer

No, I mean except 1315 and 1316.

---

Karl Cheng (qantas94heavy) - over 6 years ago

source maintainer

Which version of openSUSE is 1316?

---

Ismail Dönmez (namtrac) - over 6 years ago

@hillwood that's a good question, what is 1316 refers to ?

---

Hillwood Yang (hillwood) - over 6 years ago

author source maintainer

1316 = Leap 42.2 and 42.3

---

Dominique Leuenberger (dimstar) - over 6 years ago

reviewer

Where is that info from?

Leap 42.x is suse_version 1315

$ osc meta prjconf openSUSE:Leap:42.2 | grep suse_version
%define suse_version 1315
%suse_version 1315
$ osc meta prjconf openSUSE:Leap:42.3 | grep suse_version
%define suse_version 1315
%suse_version 1315