-
Notifications
You must be signed in to change notification settings - Fork 449
feat(BE): release pipeline v0.1.0 #5496
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
e5fab00
to
6b14e39
Compare
550f092
to
4909e38
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5496 +/- ##
========================================
Coverage 97.76% 97.77%
========================================
Files 1245 1253 +8
Lines 44062 44233 +171
========================================
+ Hits 43076 43247 +171
Misses 986 986 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall, will need to add test and coverage
9a4242a
to
7c4ef9f
Compare
for more information, see https://pre-commit.ci
c773096
to
3be9ede
Compare
b20e53b
to
9fdd219
Compare
Docker builds report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and been tested over the different frontend reviews.
One UX comment (ordering) if possible to update in private
release_pipelines_views = importlib.import_module("release_pipelines_logic.views") | ||
projects_router.register( | ||
r"release-pipelines", | ||
release_pipelines_views.ReleasePipelineViewSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, I realized this morning. Right now they are ordered by ASC created at. It might make more sense to have the newest pipelines first in GET list if not too much effort
|
||
|
||
class PipelineStage(models.Model): | ||
name = models.CharField(max_length=255) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiagoapolo, NIT detail for future versions but maybe in the frontend we could warn/block same stage names in a pipeline
Thanks for submitting a PR! Please check the boxes below:
docs/
if required so people know about the feature!Changes
Related Epic: #5486
How did you test this code?
Add new unit test here and in the private repo