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

fix RuboCop #545

Merged
merged 1 commit into from
Jul 17, 2024
Merged

fix RuboCop #545

merged 1 commit into from
Jul 17, 2024

Conversation

midzer
Copy link
Contributor

@midzer midzer commented Jul 15, 2024

Follow up to #544

@parkr
Copy link
Member

parkr commented Jul 17, 2024

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 5ba1121 into jekyll:master Jul 17, 2024
7 checks passed
jekyllbot added a commit that referenced this pull request Jul 17, 2024
github-actions bot pushed a commit that referenced this pull request Jul 17, 2024
midzer: fix RuboCop (#545)

Merge pull request 545
@ashmaroli
Copy link
Member

Whoa! @midzer, a couple of changes made here are incorrect and not at all equivalent to previous code. Please review changes once more.

P.S. I'm intentionally refraining from pointing the incorrect changes out in order to encourage you to identify those by yourselves.

@midzer
Copy link
Contributor Author

midzer commented Jul 20, 2024

@ashmaroli Sorry, I don't know what you mean. Can you give me a hint whats wrong with the code?

@ashmaroli
Copy link
Member

Can you give me a hint...

@midzer I am assuming that you are new to Ruby and therefore I understand that it would be a lot easier for you if I told you exactly what is incorrect. But I will still refrain from giving you the correct solution so as to encourage active knowledge-seeking from your end.

The incorrect changes are in the following:

options = { directory => opts.fetch("directory", "") }
if fd.include?(".") || fd.include?("..")

Once you realise the solutions, please open a pull request.

P.S. The test coverage for this project is scarce. So CI passing isn't a guarantee enough.

@parkr
Copy link
Member

parkr commented Jul 21, 2024

Ah, good catch @ashmaroli. I haven't made a release so any changes can be fixed before then.

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

Successfully merging this pull request may close these issues.

4 participants