Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tap2junit v0.1.5 proposal #12

Merged
merged 1 commit into from
Sep 26, 2019
Merged

tap2junit v0.1.5 proposal #12

merged 1 commit into from
Sep 26, 2019

Conversation

cclauss
Copy link
Collaborator

@cclauss cclauss commented Sep 25, 2019

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

  • Add support for Python 3
  • Add automated testing on Travis CI
  • Remove Pipfile.lock to support multiple Python versions

To be released at https://pypi.org/project/tap2junit

@jbergstroem
Copy link
Member

Looks like some .py files got an executable bit (0644 -> 0755). Is this intentional?

@sam-github
Copy link

@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.

@cclauss
Copy link
Collaborator Author

cclauss commented Sep 25, 2019

I do NOT run Windows. I am a Mac user. I flipped those bit for local tests so I have unfliped them.

Copy link

@sam-github sam-github left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/! :=-)

#
# This program is free software; you can redistribute it and/or modify

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't relicense this code unless we own the copyright to it, so please revert this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is a reference to my previous work I'm happy to transfer/donate ownership.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you didn't write tap13.py, its +# Author: Josef Skladanka <[email protected]>

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 ?

Copy link

@sam-github sam-github Sep 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was all the other code in your initial commit yours, @jbergstroem ?

The glue/sauce was mine, yeah.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

test/fixtures/test2.xml Outdated Show resolved Hide resolved
@sam-github
Copy link

@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?

@cclauss
Copy link
Collaborator Author

cclauss commented Sep 25, 2019

OK. Will split into two PRs.

@@ -1,27 +1,30 @@
#!/usr/bin/env python3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@@ -1,31 +1,37 @@
from __future__ import print_function
#!/usr/bin/env python3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above.

@cclauss cclauss added the wip Work in Progress: Do not merge label Sep 25, 2019
@cclauss cclauss closed this Sep 26, 2019
@cclauss cclauss reopened this Sep 26, 2019
@cclauss cclauss added ready for review and removed wip Work in Progress: Do not merge labels Sep 26, 2019
@cclauss cclauss merged commit 9da46c1 into nodejs:master Sep 26, 2019
@cclauss cclauss deleted the release-0.1.5 branch September 26, 2019 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants