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.

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

Change path_fixes to report_fixes #224

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

joseph-sentry
Copy link
Contributor

@joseph-sentry joseph-sentry commented Jul 28, 2023

Fixes: #207

Depends on: codecov/worker#58

@joseph-sentry joseph-sentry marked this pull request as ready for review August 2, 2023 15:09
Copy link
Contributor

@dana-yaish dana-yaish left a comment

Choose a reason for hiding this comment

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

can you elaborate more on why we're doing this? I can see that worker is expecting path_fixes to be in the stored report
https://github.com/codecov/worker/blob/main/services/report/parser/version_one.py#L27

@joseph-sentry
Copy link
Contributor Author

Hm, it looks like the renaming will have to happen in the worker as well. @drazisil-codecov pointed out that what is happening in _get_file_fixers is not path fixing, it's report fixing

@dana-yaish
Copy link
Contributor

but I can see that the legacy way to get and build path fixes in worker is the same in the CLI, is this a matter of naming only?

@joseph-sentry
Copy link
Contributor Author

but I can see that the legacy way to get and build path fixes in worker is the same in the CLI, is this a matter of naming only?

Yes, for the most part I think this is a naming issue, and it seems to exist in the uploader/legacy version as well

I am curious to know if we should be using this:
https://github.com/codecov/worker/blob/main/services/path_fixer/fixpaths.py#L8
in the processing of a V1 report because I think it's only being used in the processing of a legacy report and I'm not sure whether it's actually necessary for a V1 report. I think this line is part of default path fixing

@joseph-sentry joseph-sentry marked this pull request as draft August 9, 2023 14:59
@joseph-sentry joseph-sentry marked this pull request as ready for review August 11, 2023 16:11
@joseph-sentry
Copy link
Contributor Author

joseph-sentry commented Sep 12, 2023

This shouldn't be a breaking change because the worker now handles the field being named "path_fixes" and "report_fixes" in codecov/worker#58, which this PR depends on.

@thomasrockhu-codecov
Copy link
Contributor

@joseph-sentry is there any update on this?

@joseph-sentry
Copy link
Contributor Author

@joseph-sentry is there any update on this?

This is ready for another review

@joseph-sentry joseph-sentry force-pushed the joseph/path-fixes-to-report-fixes branch from 71a1eac to cecd83f Compare October 10, 2023 12:16
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #224 (5726dc4) into main (73f09c4) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #224   +/-   ##
=======================================
  Coverage   95.55%   95.55%           
=======================================
  Files          79       79           
  Lines        2745     2745           
=======================================
  Hits         2623     2623           
  Misses        122      122           
Flag Coverage Δ
python3.10 95.67% <ø> (ø)
python3.11 95.66% <ø> (ø)
python3.8 95.67% <ø> (ø)
python3.9 95.67% <ø> (ø)
smart-labels 95.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
codecov_cli/services/upload/upload_sender.py 100.00% <ø> (ø)

@joseph-sentry joseph-sentry force-pushed the joseph/path-fixes-to-report-fixes branch 3 times, most recently from 9261561 to ced40e9 Compare October 11, 2023 11:58
@joseph-sentry joseph-sentry force-pushed the joseph/path-fixes-to-report-fixes branch from ced40e9 to bf9c4bb Compare October 12, 2023 10:28
@thomasrockhu-codecov
Copy link
Contributor

@joseph-sentry any movement here?

@joseph-sentry joseph-sentry force-pushed the joseph/path-fixes-to-report-fixes branch from bf9c4bb to e4b209e Compare October 23, 2023 13:43
@joseph-sentry joseph-sentry force-pushed the joseph/path-fixes-to-report-fixes branch from e4b209e to 5726dc4 Compare October 23, 2023 15:14
@joseph-sentry joseph-sentry merged commit 775ba46 into main Oct 23, 2023
14 checks passed
@joseph-sentry joseph-sentry deleted the joseph/path-fixes-to-report-fixes branch October 23, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI labels the "report_fixes" section of the upload as "path_fixes"
5 participants