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

Move NodePattern into AST namespace #7

Closed
Drenmi opened this issue May 14, 2020 · 9 comments
Closed

Move NodePattern into AST namespace #7

Drenmi opened this issue May 14, 2020 · 9 comments
Assignees

Comments

@Drenmi
Copy link
Contributor

Drenmi commented May 14, 2020

Now that we have a separate gem for rubocop-ast, maybe it's time we move NodePattern out of the RuboCop namespace and into the gem namespace. WDYT? 🙂

@bbatsov
Copy link
Contributor

bbatsov commented May 14, 2020

I was about to file the same ticket myself. :D Eventually we should move everything that's under RuboCop currently to RuboCop::AST. We have to be smart about this, however, as it's going to cause breakages - one option would be to utilize some aliasing.

@bbatsov
Copy link
Contributor

bbatsov commented May 14, 2020

It's also a bit weird that RuboCop::Error ended up in rubocop-ast. Probably we should just use a different constant in rubocop and rubocop-ast, but that's definitely not a big deal.

@marcandre
Copy link
Contributor

Looks like a good idea to move NodePattern, ProcessedSource and Token into AST.

Probably best to create the aliases in rubocop gem itself... I'll take care of that.

We could do the same for RuboCop::Error, but I feel it's less sensical. Would be strange that ConfigNotFoundError would be a AST::Error...

I think it's a principle to have a common base class for errors. I don't think we need a rubocop-core/tools/base gem just for that and a few odds and ends, but I'm not against it either.

An alternative solution would be to have RuboCop::AST::Error and have RuboCop::Error be a module that we include (instead of derive from) in rubocop; it would be included in AST::Error from rubocop gem. This way rescue RuboCop::Error would still rescue both AST::Error and ConfigNotFoundError.

@marcandre marcandre self-assigned this May 14, 2020
@marcandre
Copy link
Contributor

Forgot to mention, but ValidationError doesn't belong in rubocop-ast, my bad. I'll fix that too.

@marcandre
Copy link
Contributor

Nevermind, Error & al. can be moved no problem, my bad.

@bbatsov
Copy link
Contributor

bbatsov commented May 15, 2020

👍

Okay, I guess once this is done we can bump rubocop-ast to 1.0.

marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
…bocop/token_spec.rb into ast/

Done in two operations to facilitate git-blame
marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
…bocop/token_spec.rb into ast/

Done in two operations to facilitate git-blame
marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
marcandre added a commit to marcandre/rubocop that referenced this issue May 15, 2020
marcandre added a commit to marcandre/rubocop that referenced this issue May 15, 2020
marcandre added a commit to marcandre/rubocop that referenced this issue May 15, 2020
marcandre added a commit to rubocop/rubocop that referenced this issue May 15, 2020
marcandre added a commit to rubocop/rubocop that referenced this issue May 15, 2020
marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
…bocop/token_spec.rb into ast/

Done in two operations to facilitate git-blame
marcandre added a commit to marcandre/rubocop-ast that referenced this issue May 15, 2020
@marcandre marcandre mentioned this issue May 15, 2020
marcandre added a commit that referenced this issue May 15, 2020
…oken_spec.rb into ast/

Done in two operations to facilitate git-blame
@marcandre
Copy link
Contributor

I'll release 0.0.3 for now... and we can release 1.0 at the same time as rubocop?

@bbatsov
Copy link
Contributor

bbatsov commented May 15, 2020

Sounds good. I guess I can now cut one extra RuboCop release to see if we didn't break anything with the extraction for some external gem.

@marcandre
Copy link
Contributor

That's something I want to add to the github actions... testing with earliest compatible rubocop release (not just master)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants