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

[MSHARED-862] system independent file separator #21

Merged
merged 6 commits into from
Mar 12, 2020
Merged

[MSHARED-862] system independent file separator #21

merged 6 commits into from
Mar 12, 2020

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Mar 11, 2020

@rfscholte see if we can make DirectoryScannerTest pass on Windows

@elharo elharo requested a review from rfscholte March 11, 2020 20:29
@elharo elharo changed the title system independent file separator [MSHARED-862] system independent file separator Mar 11, 2020
@michael-o
Copy link
Member

Let me run this too...

Copy link
Contributor Author

@elharo elharo left a comment

Choose a reason for hiding this comment

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

I'm a little confused why the jenkins build isn't firing on the latst commits:

https://builds.apache.org/job/maven-box/job/maven-shared-utils/

Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

Works for me on

Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T20:33:14+02:00)
Maven home: /usr/local/apache-maven-3.5.4
Java version: 1.8.0_242, vendor: Oracle Corporation, runtime: /usr/local/openjdk8/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "freebsd", version: "12.1-stable", arch: "amd64", family: "unix"

and

Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T20:33:14+02:00)
Maven home: C:\Entwicklung\Programme\apache-maven-3.5.4\bin\..
Java version: 1.8.0_242, vendor: Azul Systems, Inc., runtime: C:\Program Files\Zulu\zulu-8\jre
Default locale: de_DE, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

Suprisingly I see an unrelated failure:

[ERROR] Failures:
[ERROR]   MessageUtilsTest.testSystem:39
Expected: not sameInstance(<org.apache.maven.surefire.report.ConsoleOutputCapture$ForwardingPrintStream@2cfb4a64>)
     but: was <org.apache.maven.surefire.report.ConsoleOutputCapture$ForwardingPrintStream@2cfb4a64>

@elharo
Copy link
Contributor Author

elharo commented Mar 12, 2020

Interesting. I've never seen that one. Do you also see that one in master?

@michael-o
Copy link
Member

Interesting. I've never seen that one. Do you also see that one in master?

Yes, same on master.

@elharo
Copy link
Contributor Author

elharo commented Mar 12, 2020

Build is blue (green) so I'm going to merge.

@elharo elharo merged commit 5bad9d9 into master Mar 12, 2020
@elharo elharo deleted the windows branch March 12, 2020 14:45
@michael-o
Copy link
Member

Ouch, you just have provided 6 junk commits on master. Please avoid that by squashing first. No merge commit since they don't add any benefit for us.

@elharo
Copy link
Contributor Author

elharo commented Mar 12, 2020

Squash commits have a different negative side effect that Robert has asked me to avoid. You should probably bring this up on the dev mailing list, so the community can decide what they want. When a decision is made, the repos can be configured to make the preferred way the only way.

@michael-o
Copy link
Member

Where did he request this?

@elharo
Copy link
Contributor Author

elharo commented Mar 12, 2020

Slack + email. I don't have a major preference here one way or the other, but I am getting contradictory requests about merging from different committers and PMC members, so let's figure out what everyone is most comfortable with, document that decision, and configure the repos to make it the default or only option.

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