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 erroneous cosmetic changes in Pebble importer #546

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

midzer
Copy link
Contributor

@midzer midzer commented Jul 21, 2024

(Trying) to fix #545

@parkr parkr requested a review from ashmaroli July 21, 2024 18:50
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

One looks right to me but the other needs another look. Thanks for playing along!

lib/jekyll-import/importers/pebble.rb Show resolved Hide resolved
lib/jekyll-import/importers/pebble.rb Outdated Show resolved Hide resolved
@ashmaroli
Copy link
Member

and both ways appear to match desired file names.

@midzer :) the thing is that both forms are 99% equivalent. The gotcha with Ruby compared to Python is that one can have multiple ways to implement the same idea. Therefore, if something; next item; end is almost equivalent to next item if something.

What Parker highlighted in his explanation is that creating the array [".", ".."] for each iteration is waste of resources, albeit hardly noticeable in most situations. To that end, your suggestion to use Dir.each_child is indeed a superior alternative. It's just that we would have to be certain that subdirectories are handled properly as well (Disclaimer: I'm not totally familiar with the pebble platform or the importer code).

@ashmaroli ashmaroli changed the title fix Pebble Regression Fix erroneous cosmetic changes in Pebble importer Jul 23, 2024
@ashmaroli
Copy link
Member

LGTM!

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit a8a04c2 into jekyll:master Jul 23, 2024
7 checks passed
jekyllbot added a commit that referenced this pull request Jul 23, 2024
github-actions bot pushed a commit that referenced this pull request Jul 23, 2024
midzer: Fix erroneous cosmetic changes in Pebble importer (#546)

Merge pull request 546
@ashmaroli
Copy link
Member

Thank you @midzer. :)

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