-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Extensible Homebrew DSLs #7984
Comments
Those who agree with this proposal: what location do you think makes sense to use?
Are there any obvious drawbacks? |
Is just using This way, if anything is going to need these features, the line to use is Putting things in I don't know if there's a convention to follow here, but that's my gut feeling. |
I think putting stuff in a |
I agree that it seems logical to use |
I agree with this when multiple classes are used. I disagree when the definition of a class is split across multiple files. |
What is your concern exactly? From what I see online, people do this in Ruby quite frequently and I think |
Works for me.
I have found the opposite to be the case. I'd ideally like to see Homebrew have each class be defined in a file with the equivalent name. If This isn't a speculative problem, either, I have (regularly) found this a problem with other Ruby projects. I think smaller classes are better but I don't think a long file is inherently problematic if it's due to being a class that's actually got a lot of public methods. |
Well, right now one has to search in a file to find that method. If we don't do anything special, one would have to require "something"
# Defines instance methods:
# - skip_clean
require "something_else"
# Defines class methods:
# - sad_panda? We would then be able to provide in-depth comments in the "required" files. I'm not sure if this is something you want to hear more opinions about or not. If not -- I'm OK with closing this issue. |
These will become out of date.
If you're OK with that: I think that'd be better, sorry 😭. |
No worries! This is just a suggestion. ✌️ @Rylan12 @Bo98 @SeekingMeaning @jonchang @dawidd6, I'm ready to close this so if you think there is an important argument I missed -- please post it today. Otherwise, I'll close this issue in a few hours. |
The only thing I would add is that I think we should exclude Actually, I'm not sure what the reasoning behind the limit is in general. I would think it is a way to say "hey, this class is super long so let's try to break it up a little bit" but, based on this thread, it seems like we would prefer not breaking it up even for longer files. Maybe |
Done: #8209
I think |
Good point, Rylan. I think the situation is a bit clearer now, so this issue can be closed. |
Description
I propose to start splitting various independent pieces of various DSLs in Homebrew into smaller files which we can then "require" in the corresponding "main" DSL file.
For example:
formula.rb
provides a DSL for Formula object. Some features are completely independent of others, such as:skip_clean
,link_overwrite
,ignore_missing_libraries
, ... we can factor these out into files inextend/formula/skip_clean.rb
,extend/formula/link_overwrite.rb
andextend/formula/ignore_missing_libraries.rb
and require those informula.rb
.Motivation
As I was working on "missing libraries" feature (PR 7847), my changes to formula.rb made it longer than 1400 lines, which causes style check to fail. I guess it's possible to increase this limit somewhere, but I think that proposed approach is better in that it would also enable us to quickly and conditionally turn certain features on and off when desired
Relevance
Alternatives
Keep using current files and increase the number-of-lines limit for all files in audit checks.
The text was updated successfully, but these errors were encountered: