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

Provide new feature flag context for devices #5226

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Provide new feature flag context for devices #5226

merged 1 commit into from
Jan 8, 2025

Conversation

withinfocus
Copy link
Contributor

@withinfocus withinfocus commented Jan 7, 2025

🎟️ Tracking

Via Slack conversations.

📔 Objective

Adds a new context "kind" in LD for devices. When a device identifier is present, add the context so that targeting can be used against it -- this is needed for when a user has yet to be established so that context will not be available for anything other than the anonymous catch-all user.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@withinfocus withinfocus marked this pull request as ready for review January 7, 2025 15:57
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.71%. Comparing base (2a6abb9) to head (0aaf604).
Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5226   +/-   ##
=======================================
  Coverage   43.70%   43.71%           
=======================================
  Files        1472     1472           
  Lines       67963    67970    +7     
  Branches     6161     6162    +1     
=======================================
+ Hits        29703    29710    +7     
  Misses      36963    36963           
  Partials     1297     1297           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@withinfocus withinfocus requested review from a team and addisonbeck January 7, 2025 15:57
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Logo
Checkmarx One – Scan Summary & Detailsa1152d6e-c149-4574-ac69-e35daf8ea965

No New Or Fixed Issues Found

Comment on lines +19 to 21
private const string _contextKindDevice = "device";
private const string _contextKindOrganization = "organization";
private const string _contextKindServiceAccount = "service-account";
Copy link
Contributor

Choose a reason for hiding this comment

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

At what point would we want to make this an enum/class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I think I am done adding them and we can't invent any more, then another comes up. This is (probably) the last one! 😜

@withinfocus withinfocus merged commit 92d9b88 into main Jan 8, 2025
53 checks passed
@withinfocus withinfocus deleted the ld-device branch January 8, 2025 18:54
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.

2 participants