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: add semver constraint for git storage #2977

Merged
merged 15 commits into from
Apr 30, 2024
Merged

feat: add semver constraint for git storage #2977

merged 15 commits into from
Apr 30, 2024

Conversation

thepabloaguilar
Copy link
Contributor

This PR introduces the ability to define semantic versioning for Git storage!

Sorry for the tests, I've tried to think how I could improve but nothing came to my mind so I just copied the other test and adapted for the implementation change

Closes #2905

@thepabloaguilar thepabloaguilar requested a review from a team as a code owner April 12, 2024 02:56
@thepabloaguilar thepabloaguilar force-pushed the issue-2905 branch 2 times, most recently from 1f524e7 to 29b130c Compare April 12, 2024 02:59
@codecov Codecov
Copy link

codecov bot commented Apr 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 81.39535% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 72.24%. Comparing base (f997fb9) to head (60cf3bc).
Report is 280 commits behind head on main.

Files Patch % Lines
internal/storage/fs/git/reference_resolvers.go 74.19% 4 Missing and 4 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2977      +/-   ##
==========================================
+ Coverage   70.78%   72.24%   +1.45%     
==========================================
  Files          91       98       +7     
  Lines        8729     7425    -1304     
==========================================
- Hits         6179     5364     -815     
+ Misses       2165     1661     -504     
- Partials      385      400      +15

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

markphelps
markphelps reviewed Apr 29, 2024
View reviewed changes
Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

one minor ask about adding some test code to test we correctly skip non-sem ver tags, but otherwise LGTM!!

/cc @erka @GeorgeMac

internal/storage/fs/git/reference_resolvers_test.go Show resolved Hide resolved
@thepabloaguilar
feat: add semver constraint for git storage
40e643e 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: go mod tidy
0153f4c 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: ignore linter issue
aa3eea4 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
fix: ref specs
0c57ff9 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
fix: tag export information
402a537 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
fix: test wait for snapshot to be refreshed
d4a2c1f 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: add ref_type to config schemas
5bb6eca 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
feat: add RefType to Git storage struct
27a3edc 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
feat: add reference resolver functions for modularity of git hash res…
38d14cf 
…olvers

Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: improve resolvers tests
c49a735 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: change string to correct type
30ee813 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: go mod tidy
c5f676a 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: fix lint
16c88f3 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: add the author on tests commits
1ca839c 
Signed-off-by: Pablo Aguilar <[email protected]>
@thepabloaguilar
chore: add test to ensure non compliant tags do not break
60cf3bc 
Signed-off-by: Pablo Aguilar <[email protected]>
GeorgeMac
GeorgeMac approved these changes Apr 30, 2024
View reviewed changes
Copy link
Member

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

👏 very nice. Thanks for such a great improvement!

markphelps
markphelps approved these changes Apr 30, 2024
View reviewed changes
Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

Thank you!

@markphelps markphelps added the automerge Used by Kodiak bot to automerge PRs label Apr 30, 2024
@kodiakhq kodiakhq bot merged commit 571c644 into flipt-io:main Apr 30, 2024
28 checks passed
@markphelps markphelps added the needs docs Requires documentation updates label Apr 30, 2024
@thepabloaguilar thepabloaguilar deleted the issue-2905 branch April 30, 2024 16:58
@markphelps markphelps removed the needs docs Requires documentation updates label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markphelps markphelps markphelps approved these changes

@GeorgeMac GeorgeMac GeorgeMac approved these changes

@erka erka Awaiting requested review from erka

Assignees
No one assigned
Labels
automerge Used by Kodiak bot to automerge PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support semantic versioning on Git declarative storage
4 participants
@thepabloaguilar @erka @markphelps @GeorgeMac