-
Notifications
You must be signed in to change notification settings - Fork 533
Creates CONTRIBUTOR and MAINTAINERS files #353
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
Conversation
…is written in * adding CONTRIBUTING and MAINTAINERS markdown file based on projects that I have worked it (discussion still open for final) * adding new files and extension to gemspec Signed-off-by: Vern Burton <me@vernburton.com>
Signed-off-by: Vern Burton <me@vernburton.com>
Signed-off-by: Vern Burton <me@vernburton.com>
Signed-off-by: Vern Burton <me@vernburton.com>
Signed-off-by: Vern Burton <me@vernburton.com>
The failure of jruby-9.1.15.0 in Travis CI seems to be a random occurrence. My change doesn't even change how the code works but it failed the test. I would propose that we allow failures of jruby-9.1.15.0 in Travis. What do you think @perlun? |
You're right, it seems to be an intermittent error. I restarted the job now; it seemed to work better this time. JRuby threads are much more "real" threads than MRI (because of no GIL), so the level of concurrency is higher; that's likely to be the source of the difference here.
I am hesitant to do that, since it would easily allow us to miss real JRuby breakage (i.e. things that for whatever reason doesn't work there.) I'm a long-time JRuby user myself, and I know how annoying it is with gems that only work on MRI, so for the time being, I think it would be much better to look into the root cause for that intermittent error. There is an issue about it: #319 Will get back to reviewing the rest ASAP, it requires a bit more thought & time. |
I will rebase the branch when I get home. |
CONTRIBUTING.md
Outdated
|
||
### Submitting Issues | ||
|
||
We utilize **Github Issues** for issue tracking and contributions. You can contribute in two ways: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: Github -> GitHub, both here and in the other places in the document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do tonight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some open issues I'd like to see addressed before merge.
sign-off) is maintained indefinitely and may be redistributed | ||
consistent with this project or the open source license(s) | ||
involved. | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rvodden I'm not so sure we need the DCO. It feels a bit far-fetched to me. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm with you, in that it is a bit far fetched, but on the other hand all the OS projects i work on them have them... its only a check box, and I don't think its much to ask; On the other hand if we miss it out and have no legal protection at all we risk (admittedly and extremely low risk) losing all our work. I'm minded to leave it in...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree about the legal implications, they are valid. If it was only a check box, I would be fine with it, but as we have it written now:
In the commit message of the contribution, the developer simply adds a Signed-off-by statement and thereby agrees to the DCO, which you can find below or at http://developercertificate.org/.
...it means more work every time someone makes a single commit and submits it to the project. (OTOH, you can automate it by using git commit -s
or --sign-off
, but still...)
I think, if we rephrase this to be just a mere checkbox in the PR, I am fine with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@perlun I would be hesitate to change the standard for DCO without understand the legal implications that change could introduce. The DCO depends on the git Sign-Off-By because of the integrity that is built into Git, removing that could nullify the protections that are given to the maintainers of the project.
The IDE that I use has the ability to sign-off a commit built-in, but I do not believe that the addition of a -s
in a command line string is asking too much either. GitHub has tools that can check for DCO on a commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IDE that I use has the ability to sign-off a commit built-in, but I do not believe that the addition of a -s in a command line string is asking too much either. GitHub has tools that can check for DCO on a commit.
Alright, I'm willing to give it a try. We can drop this requirement later if it turns out to be too burdensome.
PULL_REQUEST_TEMPLATE.md
Outdated
### Your checklist for this pull request | ||
🚨Please review the [guidelines for contributing](../CONTRIBUTING.md) to this repository. | ||
|
||
- [ ] Ensure all commits include |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should have been the DCO sign-off, but could be rendered moot by your other comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@perlun My branch has been rebased and comments addressed.
The thread safety issue has also struck on the Ruby 2.1.10 inside of Travis.
* updating Github to GitHub * completing sentence for DCO signoff Signed-off-by: Vern Burton <me@vernburton.com>
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@tarcinil I merged this now. Please add the tool in GitHub that checks for DCO so we don't have to check that manually. |
@perlun I would love to but I believe I would need to be added to the organization so that I can configure integrations. Please see the integration located here: Installation can be done through: |
@tarcinil Sure, I'll add you - we need all the help we can get. Visit this page and you should be able to see your invitation: https://github.com/orgs/ruby-git/invitation |
@tarcinil Great. I approved the request now, thanks for your efforts. |
Created to create a standard review expectation for potential contributors and a clear representation of maintainers.
The MAINTAINERS file will help avoid issues like those created in #342 (comment)
closes #349