-
Notifications
You must be signed in to change notification settings - Fork 27
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
⬆️ Maintenance/week 12 test, tooling, libs/packages #2916
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2916 +/- ##
========================================
- Coverage 79.5% 79.5% -0.1%
========================================
Files 677 677
Lines 28231 28230 -1
Branches 3282 3643 +361
========================================
- Hits 22469 22461 -8
- Misses 4995 5002 +7
Partials 767 767
Flags with carried forward coverage won't be shown. Click here to find out more.
|
services/director-v2/tests/unit/with_dbs/test_route_clusters_details.py
Outdated
Show resolved
Hide resolved
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.
this is the most beautiful thing I've ever seen
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.
just revert the flaky test that is not anymore flaky.
Also I am not sure that table is very useful in the way it is at the moment. like httpx removed ok... but that's not true.
services/director-v2/tests/unit/with_dbs/test_route_clusters_details.py
Outdated
Show resolved
Hide resolved
@GitHK FYI code ownership is defined in CODEOWNERS. |
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 I am not sure that table is very useful in the way it is at the moment. like httpx removed ok... but that's not true.
@GitHK please make sure the table makes sense!
So this must be correct at this point. |
@sanderegg @pcrespov the way the table is generated is wrong. The diff is done only for updated packages. We should generate the table differently. Will leave a note for the next iteration on how to fix it. |
My note was to make sure that the table has the correct results. There are two tables possible. The first accounts for the changes and the second shows the current state of the repo |
@sanderegg I've changed a bit the wording of the generated tables. Pleas let me know if it reads better now. |
@GitHK I don't understand what changed in the table. but the skipped tests are unskipped so I'm happy. |
Nothing changed in the tables. They are a they should be. Just the titles are different and should reflect better what information they are trying to provide:
With this in mind I am opened for better names |
@GitHK , @pcrespov |
The tables are work in progress and can definitively be improved. In any case, I think it brings already far more information that we had before. That said, I think it would be better if we explain offline what is the purpose of the info in the tables and where do we plan to go .
This should be interpreted as follows: this PR removes a dependency to IMO combining the info of both tables provide meaningful information. But I also agree that it is a lot of information and we could compact it a bit more. |
What do these changes do?
1/2 Changes to libraries (only updated libraries are included)
2/2 Repo wide overview of libraries
Related issue/s
How to test
Checklist