-
Notifications
You must be signed in to change notification settings - Fork 206
Add forced_traits as an option to Config.new
#612
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,14 +15,28 @@ module Warbler | |
| # the kind of project and how it should be packed into the jar or | ||
| # war file. | ||
| module Traits | ||
| attr_accessor :traits | ||
|
|
||
| def initialize | ||
| def initialize(forced_traits = []) | ||
| @forced_traits = forced_traits | ||
| @traits = auto_detect_traits | ||
|
Comment on lines
+19
to
20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bit of a nitpick, but perhaps we don't need to store the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did both during development 🤣. In the end I decided to assign it to an instance var in case it became useful to know what was forced later. I'm not married to it. |
||
| end | ||
|
|
||
| def auto_detect_traits | ||
| TraitsDependencyArray.new(Traits.constants.map {|t| Traits.const_get(t)}).tsort.select {|tc| tc.detect? } | ||
| all_trait_classes = Traits.constants.map { |t| Traits.const_get(t) } | ||
| sorted = TraitsDependencyArray.new(all_trait_classes).tsort | ||
|
|
||
| conflicts = @forced_traits.flat_map { |ft| ft.conflicts }.uniq | ||
|
|
||
| sorted.select do |tc| | ||
| if @forced_traits.include?(tc) | ||
| true | ||
| elsif conflicts.include?(tc) | ||
| false | ||
| elsif tc.requirements.any? && tc.requirements.all? { |req| conflicts.include?(req) } | ||
| false | ||
| else | ||
| tc.detect? | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def before_configure | ||
|
|
@@ -53,6 +67,10 @@ module ClassMethods | |
| def requirements | ||
| [] | ||
| end | ||
|
|
||
| def conflicts | ||
| [] | ||
| end | ||
| end | ||
|
|
||
| def self.included(base) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why this attribute is also in the
Traitssuper? Do we need to change both? (i'm not an expert with this particular Ruby magic)warbler/lib/warbler/traits.rb
Line 18 in cc3bce2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually removed that other one in this PR, since the Traits module never uses it. Config includes Trait, and having both (previously
attr_accessor :traits) doesn't do any harm, but is redundant.I changed it to
attr_readerso you cannot assign a new array of Traits in Config, but actually it doesn't prevent you from manipulating the existing array. This is a way that one could remove an unwanted Trait (config.traits.reject! { |t| t.is_a?(Warbler::Traits::Gemspec) }) but doing so does not remove the config changes that that Trait will already have made inbefore_configure. (I'm just thinking out loud here).