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.

Sign up for GitHub

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

Jump to bottom

feat: file extension detection #257

Merged
merged 8 commits into from
Jan 12, 2023
Merged

feat: file extension detection #257

merged 8 commits into from
Jan 12, 2023

Conversation

james-milligan
Copy link
Contributor

@james-milligan james-milligan commented Jan 10, 2023 •
edited
Loading

Signed-off-by: James Milligan [email protected]

This PR

  • file type is detected via the file extension, it is now possible to mix and match json and yaml files as flag configuration sources
start -f file:./config/samples/example_flags.yaml -f file:./config/samples/example_flags.json

Related Issues

#240

Notes

Follow-up Tasks

It is my opinion that the -e or --evaluator flag is now a little misleading / confusing, as only the json_evaluator is used internally, and changing the flag value has no effect internally. Open to opinions on how this should be handled, if at all.

How to test

@james-milligan
file extension detection
13d96af 
Signed-off-by: James Milligan <[email protected]>
@toddbaert
Copy link
Member

It is my opinion that the -e or --evaluator flag is not a little misleading / confusing, as only the json_evaluator is used internally, and changing the flag value has no effect internally. Open to opinions on how this should be handled, if at all.

I agree with this, but I'm not sure what the best approach is.

@Kavindu-Dodan
Copy link
Contributor

It is my opinion that the -e or --evaluator flag is not a little misleading / confusing, as only the json_evaluator is used internally, and changing the flag value has no effect internally. Open to opinions on how this should be handled, if at all.

I agree with this, but I'm not sure what the best approach is.

We only have one evaluator type and that's JSON (shared with yaml handling). So if we do not have any intention to introduce another evaluator, we can remove this option.

@beeme1mr
Copy link
Member

We only have one evaluator type and that's JSON (shared with yaml handling). So if we do not have any intention to introduce another evaluator, we can remove this option.

Can we think of any future scenario where we would want this? If not, I think it makes sense to deprecate this option.

@james-milligan
Copy link
Contributor Author

We only have one evaluator type and that's JSON (shared with yaml handling). So if we do not have any intention to introduce another evaluator, we can remove this option.

Can we think of any future scenario where we would want this? If not, I think it makes sense to deprecate this option.

Ive just pushed the changes to deprecate the -e/--evaluator flag, a warning is logged when it is used

@james-milligan james-milligan merged commit ca22541 into open-feature:main Jan 12, 2023
@james-milligan james-milligan deleted the file-extension-detection branch January 12, 2023 09:50
@github-actions github-actions bot mentioned this pull request Jan 12, 2023
Merged
james-milligan pushed a commit that referenced this pull request Jan 12, 2023
@github-actions
chore(main): release 0.3.1 (#253)
7a1b1ef 
🤖 I have created a release *beep* *boop*
---


##
[0.3.1](v0.3.0...v0.3.1)
(2023-01-12)


### Features

* file extension detection
([#257](#257))
([ca22541](ca22541))
* ResolveAll endpoint for bulk evaluation
([#239](#239))
([6437c43](6437c43))


### Bug Fixes

* **deps:** update module github.com/bufbuild/connect-go to v1.4.1
([#268](#268))
([712d7dd](712d7dd))
* **deps:** update module github.com/mattn/go-colorable to v0.1.13
([#260](#260))
([5b11504](5b11504))
* **deps:** update module github.com/open-feature/open-feature-operator
to v0.2.23 ([#261](#261))
([a1dd3b9](a1dd3b9))
* **deps:** update module github.com/rs/cors to v1.8.3
([#264](#264))
([0e6f2f3](0e6f2f3))
* **deps:** update module github.com/stretchr/testify to v1.8.1
([#265](#265))
([2ec61c6](2ec61c6))
* improve invalid sync URI errror msg
([#252](#252))
([5939870](5939870))
* replace character slice with regex replace
([#250](#250))
([c92d101](c92d101))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@beeme1mr beeme1mr linked an issue Jan 13, 2023 that may be closed by this pull request
[FEATURE] Detect flag configuration format from file extension #240
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skyerus skyerus skyerus approved these changes

@toddbaert toddbaert toddbaert approved these changes

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan approved these changes

@beeme1mr beeme1mr beeme1mr approved these changes

@AlexsJones AlexsJones Awaiting requested review from AlexsJones

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Detect flag configuration format from file extension
5 participants
@james-milligan @toddbaert @Kavindu-Dodan @beeme1mr @skyerus