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

Clean up FeatureFlagService #167

Merged
merged 3 commits into from
Jun 28, 2022

Conversation

maciej-szlosarczyk
Copy link
Contributor

This is a cleanup of feature flag service from unused code, unwanted dependencies and some minor UI/code standard fixes.

For now the UI looks like this, I plan to update it to use the same styles as frontend in a separate PR.

Screenshot 2022-06-23 at 08 38 07

Changes

  • Remove unused deps (mailer, live dashboard)
  • Clean up compilation warnings
  • Remove unused code
  • Rename function to more Elixir-like Featureflagservice.FeatureFlags.list_feature_flags/0
  • Remove phoenix branding from UI
  • Add buttons and nicer links

* Remove unused deps (mailer, live dashboard)
* Clean up compilation warnings
* Remove unused code
* Rename function to more Elixir-like
Featureflagservice.FeatureFlags.list_feature_flags/0
* Remove phoenix branding from UI
* Add buttons and nicer links
@maciej-szlosarczyk maciej-szlosarczyk requested a review from a team June 23, 2022 05:45
@cartersocha cartersocha requested a review from tsloughter June 23, 2022 23:13
@cartersocha
Copy link
Contributor

ty for submitting @maciej-szlosarczyk. I'll check out the portal experience. @open-telemetry/demo-webstore-approvers anyone have familiarity with phoenix? From a quick review it looks like a clean up to me but I can't comment on the finer details. Adding @tsloughter to take a quick look too

Copy link
Member

@tsloughter tsloughter left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@tsloughter
Copy link
Member

For anyone else reading this, I believe there is still more that can be cleaned up, so pls send PRs if you see anything :). Main stuff I'm thinking of that can probably go is stuff out of FeatureflagserviceWeb related to liveview and maybe some of the Plug's used in the router. For example, fetch_session sounds like one we don't need.

@maciej-szlosarczyk
Copy link
Contributor Author

@tsloughter I think both have to stay - LiveView is used to render .heex templates, and session plug is required to use flash messages (put_flash(conn, :info, "Feature flag created successfully.")).

@cartersocha
Copy link
Contributor

One thing to note is we're rebranding the front end. So maybe hold off on making this too aligned or detailed with the existing front end

@tsloughter
Copy link
Member

@maciej-szlosarczyk ah! Thanks, I would have broken that :)

Copy link
Contributor

@mic-max mic-max left a comment

Choose a reason for hiding this comment

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

LGTM!

@cartersocha cartersocha merged commit 7571792 into open-telemetry:main Jun 28, 2022
GaryPWhite pushed a commit to wayfair-contribs/opentelemetry-demo that referenced this pull request Jun 30, 2022
* Remove unused deps (mailer, live dashboard)
* Clean up compilation warnings
* Remove unused code
* Rename function to more Elixir-like
Featureflagservice.FeatureFlags.list_feature_flags/0
* Remove phoenix branding from UI
* Add buttons and nicer links
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* Remove unused deps (mailer, live dashboard)
* Clean up compilation warnings
* Remove unused code
* Rename function to more Elixir-like
Featureflagservice.FeatureFlags.list_feature_flags/0
* Remove phoenix branding from UI
* Add buttons and nicer links
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.

4 participants