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

Remove predicates that apply to unsupported Rubies #2381

Closed
wants to merge 1 commit into from

Conversation

pirj
Copy link
Member

@pirj pirj commented Sep 1, 2020

No description provided.

@pirj pirj self-assigned this Sep 1, 2020
Copy link
Member

@benoittgt benoittgt left a comment

Choose a reason for hiding this comment

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

Thanks @pirj for the cleaning.

Ok for me because errors are Yard related.

@JonRowe
Copy link
Member

JonRowe commented Sep 2, 2020

I don't think we should merge this because its much easier to keep as much of the build in sync with rspec-dev as possible, yes there are some superfluous predicates but its easier to update this file with the others if one we use does change?

@pirj
Copy link
Member Author

pirj commented Sep 2, 2020

is_ruby_23_plus will become redundant when we drop support for Ruby 2.2.
additional_specs_available is unused.
We use style_and_lint_enforced only because we support more Ruby versions than our RuboCop version does. Also can be removed along with Ruby 2.2.

The only two useful ones are is_mri/is_jruby. But those two are so simple and highly unlikely to change over time.
Maybe we could get rid of them as well if as you recently mentioned we support MRI-compatible Rubies, and JRuby et al should strive for that compatibility.

@JonRowe
Copy link
Member

JonRowe commented Sep 2, 2020

Yes and when we move to RSpec 4 we can drop the functions for all repos, my point is that they are currently (to the best of my knowledge) still synced by the update pr, thus to reduce the risk of churn we can leave the unused functions.

@pirj
Copy link
Member Author

pirj commented Sep 2, 2020

The update PR applies in the part of the common plaintext files, but not travis, rubocop and scripts I believe.

Yes and when we move to RSpec 4

If it happens soon, there won't be much in that release besides dropping older Rubies 😆

Anyway, I don't mind closing this, as I work on a couple more cleanups already, it just caught my eye, and I couldn't resist sending this.

Do you guys happen to know what is causing this mountable_engine? flaky failure?

@pirj
Copy link
Member Author

pirj commented Sep 2, 2020

I was wrong with the removal of documentation_enforced predicate, we did that on purpose as Yard fails with that weird error on JRuby.

@JonRowe
Copy link
Member

JonRowe commented Sep 3, 2020

If it happens soon, there won't be much in that release besides dropping older Rubies 😆

You joke but really thats one of the main purposes of a major release for us, we've talked about that being a thing.

Anyway, I don't mind closing this, as I work on a couple more cleanups already, it just caught my eye, and I couldn't resist sending this.

Closing for now.

Do you guys happen to know what is causing this mountable_engine? flaky failure?

No but I don't think its a coincidence that we've recently merged an generator that uses this...

@JonRowe JonRowe closed this Sep 3, 2020
@pirj pirj mentioned this pull request Sep 4, 2020
53 tasks
@pirj pirj deleted the remove-unused-predicates branch November 23, 2021 18:47
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.

None yet

3 participants