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

Create Reviewed section and move AmbiguousOperator #1171

Closed
wants to merge 1 commit into from
Closed

Create Reviewed section and move AmbiguousOperator #1171

wants to merge 1 commit into from

Conversation

mxygem
Copy link
Member

@mxygem mxygem commented Aug 10, 2017

Summary

Created a 'Reviewed' section at the bottom of .rubocop_todo.yml to prevent folks from duplicating work.

Details

New to the project and the first thing I, unknowingly at the time, did was duplicate work! The first item in the todo file was Lint/AmbiguousOperator, so I did it. It wasn't until after that I realized that (1022)[https://github.com//pull/1022] solved it and it was decided not to move forward with it. Not a huge deal, but this PR looks to address that by moving items that have been decided to be kept and/or reviewed and found to be false to the bottom in their own section.

Motivation and Context

Wanting to help fix items in (1021)[https://github.com//issues/1021] without running into issues with duplicating work.

How Has This Been Tested?

I've run bundle exec rake and got the following output. (Which I'd assume is acceptable?)

cucumber-ruby: bundle exec rake
/Users/jsmith/.rbenv/versions/2.4.1/bin/ruby -I/Users/jsmith/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/rspec-core-3.6.0/lib:/Users/jsmith/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/rspec-support-3.6.0/lib /Users/jsmith/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/rspec-core-3.6.0/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
..............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................*..........................

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) Cucumber::Runtime::ForProgrammingLanguages should probably be inlined
     # Not yet implemented
     # ./spec/cucumber/runtime/for_programming_languages_spec.rb:6


Finished in 2.36 seconds (files took 0.68611 seconds to load)
553 examples, 0 failures, 1 pending

Running RuboCop...
Inspecting 342 files
......................................................................................................................................................................................................................................................................................................................................................

342 files inspected, no offenses detected
/Users/jsmith/.rbenv/versions/2.4.1/bin/ruby -S bundle exec cucumber --profile ruby
Using the ruby profile...
.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
WARNING: #Transform is deprecated and will be removed after version 2.6.0. Use ParameterType(...) instead.
(Called from /Users/jsmith/code/jaysonesmith/cucumber-ruby/tmp/aruba/features/support/transforms.rb:1:in `<top (required)>')
..............................................

118 scenarios (118 passed)
675 steps (675 passed)
0m1.630s

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@olleolleolle
Copy link
Contributor

Hi!

The workflow when using a .rubocop_todo.yml file with RuboCop is to re-apply the --auto-gen-config to re-create the .rubocop_todo.yml file.

If you did that, would you lose these manually-added changes? Would they be overwritten?

@mxygem
Copy link
Member Author

mxygem commented Aug 10, 2017

Hey, @olleolleolle, that's a good point and you're correct. Even though I know that's something that would happen, I hadn't thought about it and went with the flow as there were already items in the todo file like that.

I suppose in the cases of these items we should be moving them to .rubocop.yml so as not to be overwritten?

@olleolleolle
Copy link
Contributor

olleolleolle commented Aug 14, 2017

@jaysonesmith Now that you've lint-fixed so many things, has that file been overwritten by your changes?

Oh, there are conflicts.

Eh, perhaps this PR can be closed?

@mxygem
Copy link
Member Author

mxygem commented Aug 14, 2017

@olleolleolle It hasn't actually as I haven't re-run the generation command, I've been manually removing entries from the todo file. @nodo Do you have a preference on how you'd like changes we won't be making to be handled? (Matt referred me to you for your input)

@olleolleolle
Copy link
Contributor

@jaysonesmith Oh!

In that case, I am truly neutral.

@mxygem
Copy link
Member Author

mxygem commented Oct 3, 2017

@nodo bumping for some input here if you could, please! We need a way of notating what rubocop changes are still needed to prevent duplicate work for things that won't be changed and others.

@mxygem
Copy link
Member Author

mxygem commented Oct 6, 2017

Actually, I've decided that any style guidelines we decide against doing should be moved to the rubocop.yml file itself.

This will accomplish a few things:

  1. A central area for us all to use to track styles/cops that we've decided not to abide by.
  2. Remove styles we're not following from the _todo file, preventing duplicate work.
  3. Prevent any styles we're not following from being wiped out when --auto-gen-config is rerun to get an updated list.

I'm going to close this PR and do a new one with Ambiguous Operator moved.

@mxygem mxygem closed this Oct 6, 2017
@lock
Copy link

lock bot commented Oct 24, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants