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

More slash normalization #9

Merged
merged 3 commits into from
Mar 18, 2021
Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 18, 2021

Move normalizeSlashes() to Util\Functions class

Follow up on #5

When trying to run the full test suite on Windows, I ran into more issues with mixed slashes.

As that means that the normalizeSlashes() functionality is needed in more places, I'm proposing to introduce a generic Util\Functions class for simple static methods which are needed in multiple places.

Includes introducing dedicated tests for the normalizeSlashes() method.

I'd be happy to rename/move the class if you can think of a better name/path for it.

ViolationParser: implement use of Functions::normalizeSlashes()

Improves cross-platform compatibility of the code.

With this additional change, the tests will pass on Windows as well, meaning that this part of the functionality should now also work on Windows.

jrfnl added 2 commits March 18, 2021 07:04
Follow up on 5

When trying to run the tests on Windows, I ran into more issues with mixed slashes.

As that means that the `normalizeSlashes()` functionality is needed in more places, I'm proposing to introduce a generic `Util\Functions` class for simple static methods which are needed in multiple places.

Includes introducing dedicated tests for the `normalizeSlashes()` method.
Improves cross-platform compatibility of the code.

With this additional change, the tests will pass on Windows as well, meaning that this part of the functionality should now also work on Windows.
@afilina afilina merged commit 4ff6332 into main Mar 18, 2021
@jrfnl jrfnl deleted the feature/further-slash-normalization branch March 18, 2021 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants