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

Ignore UtilityFunction cop in all specs #138

Closed
wants to merge 2 commits into from

Conversation

pyromaniackeca
Copy link

No description provided.

@pyromaniackeca
Copy link
Author

/reviewme @rainforestapp/devs @grodowski

I also tried to see why the data_corrections aren't being excluded, but didn't have much luck, I'll probably try again in the future.

@marvin-rfbot marvin-rfbot bot added the review label Aug 19, 2022
@marvin-rfbot marvin-rfbot bot requested a review from grodowski August 19, 2022 06:25
@grodowski
Copy link
Contributor

grodowski commented Aug 19, 2022

@pyromaniackeca I found out today that reek has a bunch of issues open related to this: e.g. troessner/reek#1482.

Might have something to do with what file metadata pronto-reek passes to reek here.

@grodowski
Copy link
Contributor

grodowski commented Aug 19, 2022

The other aspect of how pronto-reek calls Reek::Examiner with a file argument might be that in this case the exclude_paths are not used at all?troessner/reek@cd46040

I read up how Reek works under the hood and I'm pretty sure the above is true: SourceLocator, which does care about the excude_paths config is never used by pronto:

  • we might want to configure excludes in pronto's config (I see you tried that in Rainforest in a commit I can't find now)
  • we might contribute to pronto-reek (and how it uses the configuration object) to improve that (if you're looking for an OSS contrib opportunity, it's here)

@pyromaniackeca
Copy link
Author

@grodowski Thanks for looking into it.

I think you're right that exclude_paths are completely ignored. In my attempts to make it work, I opened up the reek gem and set some breakpoints to try and figure out what's going on and I could not figure out where exactly it was checking the exclude_paths when run via pronto.

@pyromaniackeca
Copy link
Author

We've stopped reporting reek issues in PRs entirely, so this is no longer required.

@pyromaniackeca pyromaniackeca deleted the nk/ignore-utility-function-in-specs branch April 24, 2023 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants