Skip to content

Contributing updates v2 #1925

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Apr 24, 2018
Merged

Contributing updates v2 #1925

merged 9 commits into from
Apr 24, 2018

Conversation

benoittgt
Copy link
Member

@benoittgt benoittgt commented Dec 9, 2017 •
edited
Loading

Hello

This PR is meant to continue the work done by @samphippen on #1705. I answered to few of the comments of @ashleygwilliams and @myronmarston.

My english is not that good. So feel free to make any comments.

Their is a related PR on rspec-core rspec/rspec-core#2485 that could be added into CONTRIBUTING.md and referenced from the issue template.

On the top of CONTRIBUTING.md their is this text:

<!---
 This file was generated on 2015-12-07T22:01:06+11:00 from the rspec-dev repo.
 DO NOT modify it by hand as your changes will get lost the next time it is generated.
 -->

Is it still the case? Should I make this PR against : https://github.com/rspec/rspec-dev/blob/master/common_markdown_files/CONTRIBUTING.md.erb ?

Thanks in advance for the review.

@benoittgt
Copy link
Member Author

benoittgt commented Apr 5, 2018 •
edited
Loading

A little bump. I know you are busy but I think it should be a good idea to have this template.
Also we could close #1705

JonRowe
JonRowe requested changes Apr 5, 2018
View reviewed changes
Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

One nit pick. Otherwise looks fine.

CONTRIBUTING.md Outdated
enable us to quickly determine the issue is valid and then start debugging
within RSpec. These issues are good ones to tackle to help us actively fix bugs.

## Dev environment
Copy link
Member

Choose a reason for hiding this comment

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

This needs fleshing out :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry I have no idea for this one.

Copy link
Member

Choose a reason for hiding this comment

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

Just remove it for now

ebihara99999
ebihara99999 reviewed Apr 9, 2018
View reviewed changes
Copy link

@ebihara99999 ebihara99999 left a comment

Choose a reason for hiding this comment

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

Thank you for great work. It helps me much because I'm interested in contribution to this project.
I'd like to offer some help if you don't mind with only English correction.

CONTRIBUTING.md
### `Your first PR` issues

These issues are the ones that we be believe are best suited for new
contributors to get started on. They represent a potential meaningful
Copy link

Choose a reason for hiding this comment

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

get started on → get started

Copy link
Member

Choose a reason for hiding this comment

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

get started on is the correct english here, an alternative would be get started with.

CONTRIBUTING.md Outdated
These issues are ones that have been labelled by the maintainers that we
believe do not currently have enough information to be reproduced the RSpec
team. While not directly counted by the GitHub contribution graph, we consider
helping us to reproduced the issue with a repro case as an extremely meaningful
Copy link

Choose a reason for hiding this comment

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

helping us to reproduced the issue with a repro case would be replaced bygiving/offering the reproduction steps

Copy link
Member

Choose a reason for hiding this comment

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

Again the english is correct here, apart from a typo, s/reproduced/reproduce.

Copy link
Member

Choose a reason for hiding this comment

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

@benoittgt can we get the typo here fixed up?

CONTRIBUTING.md Outdated

### `Has reproduction case` issues

Issues that have reproduction cases have a repository that we can clone that
Copy link

Choose a reason for hiding this comment

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

This head would be better to start by These issues are the ones that... because others start like that.

Copy link

Choose a reason for hiding this comment

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

Do you mean this part as follows?

These issues are the ones that have reproduction cases, able to start working on immediately. 
These are good ones to tackle to help us actively fix bugs.

Copy link
Member

Choose a reason for hiding this comment

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

The former of your suggestions is good, I like that one

@benoittgt
Copy link
Member Author

Sorry for the delay. Still one point to fix. But I have no idea how to improve.

@ebihara99999
Copy link

Thanks for the previous commit.

Where is that? I could suggest something.

@benoittgt
Copy link
Member Author

benoittgt commented Apr 16, 2018 via email

@benoittgt
Copy link
Member Author

benoittgt commented Apr 23, 2018 •
edited
Loading

CI fail seems unrelated : https://travis-ci.org/rspec/rspec-rails/jobs/367774520

Installing rake 12.3.1
Gem::RuntimeRequirementNotMetError: rake requires Ruby version >= 2.0.0. The
current ruby version is 1.9.1.
An error occurred while installing rake (12.3.1), and Bundler
cannot continue.
Make sure that `gem install rake -v '12.3.1'` succeeds before bundling.
In Gemfile:
  rails was resolved to 3.2.22.5, which depends on
    railties was resolved to 3.2.22.5, which depends on
      rake
rake aborted!
Command failed with status (5): [./travis_retry_bundle_install.sh 2>&1...]

JonRowe
JonRowe requested changes Apr 23, 2018
View reviewed changes
Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

I kicked over the build, that one tends to be flakey, but theres still a typo to address :)

@benoittgt
Copy link
Member Author

Oups sorry. Thanks for the rebuild. It turned green.

@JonRowe JonRowe merged commit 55a15bb into rspec:master Apr 24, 2018
@JonRowe
Copy link
Member

JonRowe commented Apr 24, 2018

Thanks @benoittgt

@benoittgt benoittgt deleted the contributing-updates-v2 branch April 24, 2018 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebihara99999 ebihara99999 ebihara99999 left review comments

@JonRowe JonRowe JonRowe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benoittgt @ebihara99999 @JonRowe