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

119 Filename before the warning and full path to it #361

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

elenachavdarova
Copy link

Included full path to the file and moved the path in the start of the warning.

Fixes: #119

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request, @elenachavdarova !

Can you update the existing functional tests so they pass?

Also, what do you think about using a relative path instead of the full path? I think a relative path (relative to the directory they're in) might be a little more developer-friendly.

@elenachavdarova
Copy link
Author

Hello @danielbachhuber,
Thank you for your reply. For the full path - my opinion is that relative is better as well, however on the issue this PR is for - #119 the request is for full path (especially when automating). This is why I have set this up this way.

As for the funcitonal tests - I checked the errors upon the failed tests and I am not sure how to include the full path (as it is a tmp one) to the expected output. I have updated the PR with file name moved to the start of the error.

@danielbachhuber
Copy link
Member

Thank you for your reply. For the full path - my opinion is that relative is better as well, however on the issue this PR is for - #119 the request is for full path (especially when automating). This is why I have set this up this way.

@elenachavdarova Sounds good!

As for the funcitonal tests - I checked the errors upon the failed tests and I am not sure how to include the full path (as it is a tmp one) to the expected output. I have updated the PR with file name moved to the start of the error.

This works for me.

I'll leave this to @swissspidy for a final review. He's AFK for the next week or so though, so it might be a little bit.

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.

File name BEFORE the Warning & full path
3 participants