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

Fix typo in GridFTP server response type #7

Merged
merged 2 commits into from
Nov 30, 2017

Conversation

fscheiner
Copy link
Member

I spotted these typos by chance. As all uses of GLOBUS_GRIDFTP_SERVER_CONTROL_RESPONSE_ACCESS_DENIED originally had the same typo, there is no functional difference with the patches applied.

Signed-off-by: Frank Scheiner <scheiner@hlrs.de>
@fscheiner
Copy link
Member Author

fscheiner commented Nov 29, 2017

@matyasselmeci @bbockelm
Let's use this pull request to discuss some things:

  • Copyright statement: Although I assume a copyright statement is not really justified for such a change, I used Copyright 2017 Grid Community Forum in all cases. Do you agree with that statement for contributions and should we also add names?

  • Sign off commits: Should we require contributors to sign off patches/commits?

  • Travis CI: Although I understand that this is totally useful to have the CI build running as soon as a pull request comes in, shouldn't we make this part of the review process so outsiders cannot trigger a CI build? Or was this just triggered by my pull request because I am also a project owner (assuming this is inherited from the gridcf group)?

@brianhlin
Copy link
Member

What would be the purpose of preventing external collaborators from triggering a CI build? I think we would want all contributions to undergo the CI tests.

@matyasselmeci
Copy link
Contributor

I'm not sure if discriminating between pull requesters is even possible. What are we guarding against?

Updating the copyright is going to be a pain; even the Globus folks didn't do it consistently -- notice that the files you edited are "Copyright 1999-2006" even though they're newer than that.

The GCF isn't a legal entity so I don't know if we can assign copyright to it. We might just have to have a bunch of "Portions copyright " statements. Or maybe not; per-file copyright statements aren't really necessary. Git itself doesn't use them (consistently), nor does it have a Contributor License Agreement. If there's ever a dispute over who owns what code, we can use git blame.

Finally, I don't think an explicit git sign-off is necessary; since pretty much all changes will be applied by pull request, approving and merging the PR should already be considered a sign of approval.

@fscheiner
Copy link
Member Author

@brianhlin, @matyasselmeci :
Of course we want to test contributions, but wouldn't it be sufficient to test during a review process? I assume the resources at Travis CI are limited for free accounts and if they're blocked by pull requests we might have to wait, even if we're planning for a release. Or are limited resources no issue?

@matyasselmeci :
Yeah, maybe a single file (e.g. COPYRIGHT) in the root of the repo containing the copyright statements of all contributors could be the better solution, although changing it consistently (which years?) and by multiple contributors at the same time could also be painful. I'll update the pull request and remove the per-file copyright statements.

About the sign-offs - I'm partly with you here, but I always assumed a sign-off is more used to state that the provided code is from the contributor originally and not grabbed from someone/somewhere else.

@brianhlin
Copy link
Member

For open-source projects, we can have 5 concurrent builds at a time. That being said, if for some reason we're being blocked by PR CI, we should be able to cancel and later restart interfering builds. I'm not sure who this is limited to but it looks like as a member of the GitHub organization, I'm able to cancel/restart Travis builds.

@matyasselmeci
Copy link
Contributor

You got me curious about sign-offs so I looked up their history.

I don't think that's going to be an issue for us in this project; the Linux kernel is special in that patches can often pass through many hands before being merged, plus they don't use GitHub and PRs, but submit patches via email instead.

If we're committing on behalf of someone else, we can give them credit by specifying them as the author (--author option of git commit). Otherwise, GitHub does a good job of keeping track of the provenance of changes. So I disagree with requiring sign-offs.

@fscheiner
Copy link
Member Author

@matyasselmeci :
Oh, didn't knew that the path a patch came through was at least equally important, but makes sense from the description. Thanks for digging this out.

@msalle
Copy link
Member

msalle commented Nov 30, 2017

Hi,
concerning copyright, http://softwarefreedom.org/resources/2012/ManagingCopyrightInformation.html has some nice background information. See also https://opensource.org/faq#contributor-agreements.
I think it might in this case be best to stick to using the github logs for tracing contributions. Unless someone thinks we should become a legal entity (which would probably not even be that hard). Perhaps we should discuss further on the mailing list...

@matyasselmeci
Copy link
Contributor

matyasselmeci commented Nov 30, 2017

Any objection to me merging this as-is? At this point it's purely a code change, and as @msalle suggested, we can discuss this on the mailing list if we need to.

@matyasselmeci
Copy link
Contributor

Hearing none. Merging.

@matyasselmeci matyasselmeci merged commit 8bef7e5 into gridcf:master Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants