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

Feature and Gate Tweaks #45

Merged
merged 21 commits into from Apr 4, 2015
Merged

Feature and Gate Tweaks #45

merged 21 commits into from Apr 4, 2015

Conversation

jnunemaker
Copy link
Collaborator

Mostly internal changes but a few public ones.

  • moves typecasting to module and out of gate values
  • moves gate instrumentation to feature, will make it easier to allow tweaking gates per feature in the future
  • adds Feature#disabled_groups
  • Flipper.groups and .group_names now return a set to be consistent with intention and the rest of flipper. Perhaps I should change everything to arrays and drop the Set idea as it is kind of just annoying in ruby.

fyi @rsanheim @dewski @aroben

No need for feature name everywhere in gate. This feels like a step in right 
direction. Still more tweaks to come.
Only gate open? is instrumented and having it in gate requires passing it to 
gate from feature.
Conflicts:
	lib/flipper/feature.rb
	lib/flipper/gate_values.rb
	spec/flipper/gates/percentage_of_time_spec.rb
Seems like it could be dangerous if anything that used these modified
them in place
Not needed. Can always add back if we need to check the source of the
memory adapter.
@jnunemaker jnunemaker self-assigned this Apr 4, 2015
jnunemaker pushed a commit that referenced this pull request Apr 4, 2015
@jnunemaker jnunemaker merged commit 25ceb11 into master Apr 4, 2015
@jnunemaker jnunemaker deleted the gate-tweaks branch April 4, 2015 18:53
def percentage_of_time_value
gate_values.percentage_of_time
end

# Public: Returns the string representation of the feature.
def to_s
@to_s ||= name.to_s
name.to_s
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@rsanheim
Copy link
Contributor

rsanheim commented Apr 7, 2015

Perhaps I should change everything to arrays and drop the Set idea as it is kind of just annoying in ruby.

Agree with this, sets just don't feel needed with what we are doing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants