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

fix: correctly handle comma-separated file paths #40

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

larkinscott
Copy link
Member

Update file path processing to properly split and handle comma-separated paths, ensuring each path is treated as a separate entry when passed to the coverage tool.

Update file path processing to properly split and handle comma-separated paths,
ensuring each path is treated as a separate entry when passed to the coverage tool.
@larkinscott larkinscott force-pushed the sl/fix-path-seperator branch from 8418871 to d5324ef Compare January 14, 2025 22:16
@larkinscott larkinscott marked this pull request as ready for review January 15, 2025 14:42
Copy link
Member

@marschattha marschattha left a comment

Choose a reason for hiding this comment

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

This will be a breaking change, do we have a plan for rolling it out?

I think most of our docs and thus users use @main instead of version, maybe we want to use actual version to prevent user issues when making changes like this?

@larkinscott
Copy link
Member Author

@marschattha That's a good point and something that was discussed briefly in the ticket. We agreed that a breaking change was ok and expected with this update, but I like your idea of leaning into versioning to roll this (or other breaking changes) out more smoothly. Will have a look at that before merging.

@marschattha
Copy link
Member

I don't have much context on the background for this, but also, I was thinking, considering this is a breaking change, should we go with a deprecated warning for space separated file names?

That way, it won't straight up blow everyone's workflow who is already using it?

@larkinscott
Copy link
Member Author

@marschattha I pushed up a new commit that temporarily handles both cases and logs a deprecation warning: 509ae8f. Thoughts?

@larkinscott larkinscott force-pushed the sl/fix-path-seperator branch from 509ae8f to d853f4c Compare January 16, 2025 15:15
Copy link
Member

@marschattha marschattha left a comment

Choose a reason for hiding this comment

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

Looks great, will make rolling this out much smoother.

One minor suggestion below if that makes sense.

coverage/src/main.ts Outdated Show resolved Hide resolved
This begins to handle comma-seperated `files` input while still
supporting space-seperated `files` input, but logging a warning message
about the deprecation.
@larkinscott larkinscott force-pushed the sl/fix-path-seperator branch from d853f4c to 08a9207 Compare January 16, 2025 15:26
@larkinscott larkinscott merged commit 1fee13f into main Jan 16, 2025
1 check passed
@larkinscott larkinscott deleted the sl/fix-path-seperator branch January 16, 2025 16:54
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.

2 participants