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

Added remove_index #1

Merged
merged 6 commits into from
Sep 26, 2016
Merged

Added remove_index #1

merged 6 commits into from
Sep 26, 2016

Conversation

magec
Copy link
Contributor

@magec magec commented Sep 22, 2016

WAT

Adds a check to disallow remove_index

GIF

module Cop
module Migrations
# Your code goes here...
# Removing indexes is dangerous by nature, if the index you remove
# is used extensively, you could end up with database degradation.

Choose a reason for hiding this comment

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

That sounds to me :trollface:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hehe

Copy link
Contributor

Choose a reason for hiding this comment

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

or simply downtime :trollface:

end

end
end

Choose a reason for hiding this comment

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

What about testing when rubocop:disable Migrations/RemoveIndex comment is there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is part of rubocop code. I have testes it manually and it works, but that functionality is not related with a cop itself, but is about rubocop.

Choose a reason for hiding this comment

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

got it

@andresgutgon
Copy link

👍

@@ -1,2 +1,3 @@
--format documentation
--color
--require spec_helper
Copy link
Contributor

Choose a reason for hiding this comment

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

putting this here has bitten me tons of times, but probably someone else told you that it is awesome :trollface:

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't use it on hosted and I'm not sure it's very common...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dropped it but does not show up... wonder why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@enricostano
Copy link
Contributor

📭 👍

@magec magec force-pushed the feature/remove_index_forbidden branch from 59060e5 to b7e288c Compare September 22, 2016 14:15
module Cop
module Migrations
# Your code goes here...
# Removing indexes is dangerous by nature, if the index you remove
# is used extensively, you could end up with database degradation.
Copy link
Contributor

Choose a reason for hiding this comment

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

or simply downtime :trollface:

class RemoveIndex < Cop
MSG = 'remove_index is disalowed'.freeze
Copy link
Contributor

Choose a reason for hiding this comment

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

s/disalowed/disallowed

spec.description = ""
spec.homepage = ""
spec.license = "MIT"
spec.authors = %w(enricostano magec)
Copy link
Contributor

Choose a reason for hiding this comment

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

add your name as well

spec.add_development_dependency "bundler", "~> 1.11"
spec.add_development_dependency "rake", "~> 10.0"
spec.add_development_dependency "rspec", "3.5.0"
spec.add_development_dependency 'bundler', '~> 1.11'
Copy link
Contributor

@sauloperez sauloperez Sep 22, 2016

Choose a reason for hiding this comment

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

I believe the latest is 1.13 something. I would relax it more to be able to use the latest.

@@ -1,2 +1,3 @@
--format documentation
--color
--require spec_helper
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't use it on hosted and I'm not sure it's very common...

@franciscoj
Copy link
Contributor

👏 👍

@sauloperez sauloperez merged commit fa0e946 into master Sep 26, 2016
@sauloperez sauloperez deleted the feature/remove_index_forbidden branch September 26, 2016 09:46
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.

5 participants