Conversation
cclauss
requested review from
Trott,
bnoordhuis,
jbergstroem,
rvagg,
sam-github and
targos
|
Looks like some |
|
@cclauss are you publishing from Windows? I wonder if the tooling is assuming .py files are executable if its on a system that doesn't have an executable bit. |
|
I do NOT run Windows. I am a Mac user. I flipped those bit for local tests so I have unfliped them. |
sam-github
left a comment
sam-github
left a comment
There was a problem hiding this comment.
The copyright/license changes need to be pulled out, but otherwise this looks good to me.
LICENSE
Outdated
| @@ -0,0 +1,1505 @@ | |||
| Node.js is licensed for use as follows: | |||
There was a problem hiding this comment.
This is node's license, I don't think its appropriate here, 1. because you don't own copyright on the source, you can't re-license it, 2. because it refers to all kinds of unrelated things, like Joyent, and node's deps/! :=-)
| # Copyright 2013, Red Hat, Inc. | ||
| # Copyright Node.js contributors. All rights reserved. | ||
| # | ||
| # This program is free software; you can redistribute it and/or modify |
There was a problem hiding this comment.
We can't relicense this code unless we own the copyright to it, so please revert this.
There was a problem hiding this comment.
If that is a reference to my previous work I'm happy to transfer/donate ownership.
There was a problem hiding this comment.
I think that you didn't write tap13.py, its +# Author: Josef Skladanka <jskladan@redhat.com>
You included tap13.py in your initial commit, do you remember where it came from?
Was all the other code in your initial commit yours, @jbergstroem ?
There was a problem hiding this comment.
I don't think copyright matter as to who copyrights it, but it should be clear that all the code is copyright/written by someone (the git history makes that clear after the initial commit). Also, all files should have a license statement, and only the copyright owners can agree to the license, thus the needing to know who they are.
There was a problem hiding this comment.
Was all the other code in your initial commit yours, @jbergstroem ?
The glue/sauce was mine, yeah.
There was a problem hiding this comment.
I'm not sure if @Trott or @jasnell (who did an early audit on licenses) have an opinion, but I'd say PRing a copyright line and license into all the files that don't have one would be sufficient, or maybe a copyright and a top-level LICENSE file containing a GPL license copy? GPL would be consistent, I guess that's good, dual MIT/GPL would make it easier if we ever change the license of tap13.py with its author's permission, but honestly, rewriting in js would probably be easier than dealing with lawyers to make that happen, I can't see much point.
|
@cclauss I'm a bit confused by the mixing of actual refactors, changelog changes all under the subject of "release proposal". This isn't a release proposal, its just a PR to make useful changes to master, isn't it? Perhaps the non-changelog changes should be a seperate PR, and the changelog itself (and only itself) should be in a release-proposal PR? |
|
OK. Will split into two PRs. |
tap2junit/__main__.py
Outdated
| @@ -1,27 +1,30 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Should this perhaps be just python? Not sure how the setuptools wrapper would work, but since we support both py2 and py3 the os/distribution should likely choose what python is default?
tap2junit/tap13.py
Outdated
| @@ -1,31 +1,37 @@ | |||
| from __future__ import print_function | |||
| #!/usr/bin/env python3 | |||
cclauss
force-pushed
the
release-0.1.5
branch
from
c57b8f5 to
d9c94d4
Compare
cclauss
force-pushed
the
release-0.1.5
branch
from
f2a6a5e to
defe08d
Compare
3 participants
This will be the first release of tap2junit since its transfer into the Node.js GitHub org nodejs/admin#413.
tap2junit changelog
0.1.5
To be released at https://pypi.org/project/tap2junit