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 for #82 #83

Closed
wants to merge 2 commits into from
Closed

Fix for #82 #83

wants to merge 2 commits into from

Conversation

elv1s42
Copy link
Contributor

@elv1s42 elv1s42 commented Jun 6, 2023

TeamCity doesn't support artifacts with paths containing comma character: https://youtrack.jetbrains.com/issue/TW-19333
In order to make sure the artifacts are published correctly, we are replacing the comma character with '_'.
This PR is fix for #82

@SeanKilleen
Copy link
Member

Just out of curiosity - is there a reason we wouldn't want to add , to the _invalidFileChars at the initialization, since what we're trying to say is that the , is invalid for this purpose?

It seems like that would express the purpose clearly while saving the or statement.

@SeanKilleen
Copy link
Member

(Also, thank you for this PR!)

@elv1s42
Copy link
Contributor Author

elv1s42 commented Jun 6, 2023

Just out of curiosity - is there a reason we wouldn't want to add , to the _invalidFileChars at the initialization, since what we're trying to say is that the , is invalid for this purpose?

I agree! Changed the code a little bit. The _invalidFileChars is used only in one place, so it should not affect any other logic of the ServiceMessageFactory.

@elv1s42
Copy link
Contributor Author

elv1s42 commented Jun 6, 2023

Right now, I am testing if the fix is actually working. I can confirm it after some builds will finish in the TeamCity.
Are there any other actions required from my side to get this PR merged?

@elv1s42
Copy link
Contributor Author

elv1s42 commented Jun 7, 2023

Ok, I can confirm that the fix is working, but now I'm facing another issue: #84

@elv1s42
Copy link
Contributor Author

elv1s42 commented Jun 13, 2023

Closing this PR because it doesn't fix #84

@elv1s42 elv1s42 closed this Jun 13, 2023
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