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

[xmlreport] when a filepath is in multiple source dirs, choose the highest one #1146

Closed
wants to merge 1 commit into from

Conversation

alex
Copy link
Contributor

@alex alex commented Apr 18, 2021

fixes #578

@christopherpickering
Copy link
Contributor

@alex Will #578 (comment) sill work with your update?

@alex
Copy link
Contributor Author

alex commented Apr 22, 2021

Maybe! I'm not sure. I'm also now many yaks deep and mostly turned my attention towards: #1147

@christopherpickering
Copy link
Contributor

@alex 😁 nice! Well I put another pr for the docs.

@gro1m
Copy link

gro1m commented Feb 18, 2022

@alex Will #578 (comment) sill work with your update?

@christopherpickering Sorry for jumping in on this, but is this what is still needed to merge this PR? And if so, why is this not covered by the tests?

@christopherpickering
Copy link
Contributor

@gro1m I'm not sure, that was a long time ago.

@gro1m
Copy link

gro1m commented Feb 18, 2022

@gro1m I'm not sure, that was a long time ago.

@christopherpickering I understand that, on the other hand the change is small: the source_path is traversed bottom up from the filesystem tree, i.e. deepest level first to top-level and all tests are green. From that perspective, I do not really see what holds this back and I think to avoid ambiguity it would be really great to have the whole file path and not just the filename. So it would be great if someone could tell what is needed to merge this PR...

@gro1m
Copy link

gro1m commented Feb 21, 2022

@gro1m I'm not sure, that was a long time ago.

@christopherpickering I understand that, on the other hand the change is small: the source_path is traversed bottom up from the filesystem tree, i.e. deepest level first to top-level and all tests are green. From that perspective, I do not really see what holds this back and I think to avoid ambiguity it would be really great to have the whole file path and not just the filename. So it would be great if someone could tell what is needed to merge this PR...

Any thoughts on this @alex @nedbat ?

@nedbat nedbat force-pushed the master branch 2 times, most recently from 0c14f1a to 82169a6 Compare June 2, 2022 12:11
@ProsperousHeart
Copy link
Contributor

Is this still valid? Seems like it passed almost all checks but is a bit old

@gro1m
Copy link

gro1m commented Mar 20, 2023

Is this still valid? Seems like it passed almost all checks but is a bit old

I think so, but I am not a contributor to the project. In the end, we could also fix the issue by using the workaround in #578 (comment).

@alex
Copy link
Contributor Author

alex commented Mar 12, 2024

At this point, whatever the motivation for this is has long been overcome by events.

@alex alex closed this Mar 12, 2024
@alex alex deleted the patch-1 branch March 12, 2024 22:59
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.

Incomplete file path in XML report
4 participants