-
Notifications
You must be signed in to change notification settings - Fork 45
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
fix: hide error box when there is 0 issues 🐛 #5691
Conversation
5e79c45
to
8506033
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #5691 +/- ##
============================================
- Coverage 27.70% 27.70% -0.01%
Complexity 2165 2165
============================================
Files 1002 1002
Lines 126258 126260 +2
Branches 2580 2580
============================================
- Hits 34978 34977 -1
- Misses 89790 89793 +3
Partials 1490 1490
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
8506033
to
a724bd8
Compare
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 and if Codecov is happy, I'm too!
Please follow our commit style guide: https://osrd.fr/en/docs/guides/contribute/code/#git-commit-style |
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.
Thanks for your PR 🙏
91933b1
to
8569450
Compare
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.
Any chance you can rebase to remove irrelevant commits from this PR?
39cc040
to
ec1e301
Compare
yup @shenriotpro i made a rebase and force pushed |
Great, thanks |
ec1e301
to
87ecf68
Compare
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.
The yarn.lock update seems out of scope too.
All of these should be done in separate PRs, or at least separate commits.
2f7cbb3
to
1d6c58a
Compare
1d6c58a
to
ffa8696
Compare
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.
yop, I'm glad to see a new test file for our infra editor 💪 , can you add /editor/
in the grep
key of the playwright config in playwright.config.ts
? if you don't add it the test can't be launch
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.
Also, can you rename the files related to the tests by specifying that it is the infra editor? (and not the rolling stock editor)
bd680f3
to
b124f1e
Compare
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.
nice! 💪
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.
Hi! Indentation is not correct, it must be 2 spaces.
b124f1e
to
0c5eea6
Compare
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.
please split in 2 commits:
- one for bug correction
- one for adding test
0c5eea6
to
35b02c1
Compare
After a request from @nicolaswurtz i am splitting the PR in two, one for the feature, and one to test the feature: |
35b02c1
to
b874d01
Compare
hide messages showing there is no anomaly when there is 0 issues in the editor
close #5632