Skip to content

Conversation

Sija
Copy link
Member

@Sija Sija commented Mar 6, 2023

Files are searched for in the same order as used by RuboCop.

Resolves #275

@Sija Sija added the feature label Mar 6, 2023
@Sija Sija requested a review from veelenga March 6, 2023 04:33
@Sija Sija self-assigned this Mar 6, 2023
@Sija Sija marked this pull request as draft March 6, 2023 04:38
@Sija Sija marked this pull request as ready for review March 6, 2023 05:00
@Sija Sija requested a review from veelenga March 6, 2023 19:20
Copy link
Member

@veelenga veelenga left a comment

Choose a reason for hiding this comment

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

LGTM

@Sija
Copy link
Member Author

Sija commented Mar 6, 2023

@veelenga Should we create v1.4.3 (I guess the last in 1.4.x branch) to release it? WDYT?

@veelenga
Copy link
Member

veelenga commented Mar 6, 2023

I do not have any objections. Let's do that

@Sija Sija added this to the 1.4.3 milestone Mar 6, 2023
Copy link
Contributor

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

A more DRY approach for read_config would be something like this:

def read_config(file)
  each_config_path(file) do |path|
    if File.exists?(path)
      return File.read(path)
    end
  end
end

def each_config_path(path)
  yield path if path
  # ...
  search_paths.reverse_each do |search_path|
    yield search_path
  end
  # ...
  DEFAULT_PATHS.each do |default_path|
      yield default_path
    end
end

@Sija
Copy link
Member Author

Sija commented Mar 6, 2023

@straight-shoota Looks good, I'll do that, thanks!

@Sija
Copy link
Member Author

Sija commented Mar 6, 2023

@straight-shoota Ah, I've forgot about one thing, when path argument is given, the return should be unconditional, so that solution won't work that smoothly anymoar. EDIT: One small change we're golden :)

@Sija Sija requested a review from straight-shoota March 6, 2023 20:25
@straight-shoota
Copy link
Contributor

If you want completely different behaviour depending on whether path is present, I'd make that more explicit. Probably with two separate overloads, read_config(String | Path) and read_config(Nil) (or read_config()).

@Sija Sija merged commit 3fbbe39 into master Mar 7, 2023
@Sija Sija deleted the issue-275 branch March 7, 2023 10:11
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.

[Feature Request] Search ~/.ameba.yml as a default config for current linux user.
3 participants