search

jekyll / jekyll-import

:inbox_tray: The "jekyll import" command for importing from various blogs to Jekyll format.
https://import.jekyllrb.com
MIT License
512 stars 315 forks source link

fix RuboCop #545

Closed midzer closed 1 month ago

midzer commented 1 month ago

Follow up to https://github.com/jekyll/jekyll-import/pull/544

parkr commented 1 month ago

@jekyllbot: merge +dev

ashmaroli commented 1 month ago

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 commented 1 month ago

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

ashmaroli commented 1 month ago

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 commented 1 month ago

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