Page MenuHomePhabricator

[services][feature-flags] Query the db
ClosedPublic

Authored by tomek on Feb 23 2023, 8:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Feb 21, 2:40 PM
Unknown Object (File)
Wed, Feb 21, 2:04 PM
Unknown Object (File)
Wed, Feb 21, 2:04 PM
Unknown Object (File)
Wed, Feb 21, 2:04 PM
Unknown Object (File)
Wed, Feb 21, 2:04 PM
Unknown Object (File)
Wed, Feb 21, 2:04 PM
Unknown Object (File)
Wed, Feb 21, 2:03 PM
Unknown Object (File)
Mon, Feb 12, 3:57 AM
Subscribers

Details

Summary

The final diff about accessing the database.

Depends on D6856

Test Plan

Create some dummy data in the db. Query the db, log the result and verify that the parsed data is correct.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested review of this revision.Feb 23 2023, 8:34 AM
bartek added inline comments.
services/feature-flags/src/database.rs
209–210 ↗(On Diff #23011)

Maybe we should extract these strings as constants?

pub const PLATFORM_IOS: &str = "IOS";

Or even do an enum implementation

impl fmt::Display for Platform {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        fmt::Debug::fmt(self, f)
    }
}

// then
let platform_value = platform.to_string()

But this might be an overkill, it trades simplicity for flexibility/extensibility

This revision is now accepted and ready to land.Feb 24 2023, 2:22 AM
services/feature-flags/src/database.rs
209–210 ↗(On Diff #23011)

Agree that extracting as constants makes sense. And also, I don't think that introducing more complicated approach is a good idea - it's just too complicated for such a simple thing.

Rebase and introduce platform constants

This revision was automatically updated to reflect the committed changes.