[Issue #141] Fixing Issues and Adding Configurable Nx LGTM and LGTM Regex Options, Adding Rspec Tests#161
Conversation
simonzhu24
commented
Apr 12, 2016
simonzhu24
commented
- Adding 2x LGTM and last comment must be LGTM option
- Made configurable number of LGTM options vs "all-reviewers-must-approve" option in "~/.gitconfig.reflow" and "setup.rb"
- Made LGTM Regex Expression configuration in ~/.gitconfig.reflow and "setup.rb"
- Added Rspec tests to cover both options, checking for correct pull_request.build.target_url values as well
- Added Rspec tests to check for different LGTM expressions (found and fixed bug)
|
LGTM 👍 |
| **/.DS_Store | ||
| .rspec | ||
| gemfiles | ||
| .ruby-version |
There was a problem hiding this comment.
I think we'd like to keep this in as it's helpful to ensure development against the latest working version of ruby. Objections? I'm curious what issues you had with this in place.
There was a problem hiding this comment.
Makes sense, I don't see any reason not to keep *.ruby-version files in =)
There was a problem hiding this comment.
We added it to .gitignore because we did not want to prescribe a ruby version for you. Should it be 2.3.0 then?
There was a problem hiding this comment.
It should, quickly checking our circle file I see we are only testing against 2.2.4. Let's set it at that and I'll add a task to update our CI. I think I tested it manually against 2.3.0 when it was first released in December and it was failing.
|
Really great work here @pboling! I really like the configuration chooser at setup 😄 |
|
Regarding the spec syntax update, transpec can do the update automatically IIRC. |
|
@pboling great idea. I've created a separate task to address that to keep the changes here smaller. Also, I appologize to @simonzhu24 for giving @pboling credit for this! My activity feed made it seem like he was the author. Thank you so much @simonzhu24 for your effort here 😄 |
|
@simonzhu24 has been the primary author here, I've just been giving him a lot of feedback, as we sit next to each other in the office. 👍 |
[Issue #141] Add Configurable Nx LGTM and LGTM Regex Options1. Adding 2x LGTM and last comment must be LGTM option 2. Made configurable number of LGTM options vs "all-reviewers-must-approve" option in "~/.gitconfig.reflow" and "setup.rb" 3. Made LGTM Regex Expression configuration in ~/.gitconfig.reflow and "setup.rb" 4. Added Rspec tests to cover both options, checking for correct pull_request.build.target_url values as well 5. Added Rspec tests to check for different LGTM expressions (found and fixed bug)
simonzhu24
force-pushed
the
sz-issue/141-2xLGTM
branch
4 times, most recently
from
262435f to
4ca4347
Compare
| } | ||
| end | ||
|
|
||
| GitReflow::Config.add "constants.approval_regex", /lgtm|looks good to me|[:\+1:]|:thumbsup:|:shipit:/i |
There was a problem hiding this comment.
@simonzhu24 The regex is duplicated. We need to reference a single value everywhere the default is used. Please update this to reference GitReflow::GitServer::PullRequest::APPROVAL_REGEX
simonzhu24
force-pushed
the
sz-issue/141-2xLGTM
branch
from
4ca4347 to
f80d3e7
Compare
3 participants
