Skip to content

[#2079] fix(Files): Change the method to compare if two file refers to the same one#1120

Merged
xael-fry merged 1 commit intoplayframework:masterfrom
xael-fry:2079_fileComparaison
Mar 28, 2017
Merged

[#2079] fix(Files): Change the method to compare if two file refers to the same one#1120
xael-fry merged 1 commit intoplayframework:masterfrom
xael-fry:2079_fileComparaison

Conversation

@xael-fry
Copy link
Member

@xael-fry xael-fry commented Mar 9, 2017

@xael-fry xael-fry self-assigned this Mar 9, 2017
@xael-fry xael-fry added the defect label Mar 9, 2017
@xael-fry xael-fry added this to the 1.5.0 milestone Mar 9, 2017
@xael-fry xael-fry force-pushed the 2079_fileComparaison branch 7 times, most recently from 60074d1 to f774bf0 Compare March 10, 2017 09:45
@xael-fry xael-fry added has-pr and removed has-pr labels Mar 13, 2017
@xael-fry xael-fry requested a review from asolntsev March 13, 2017 03:59
public static void kill(String pid) throws Exception {
String os = System.getProperty("os.name");
String command = (os.startsWith("Windows")) ? "taskkill /F /PID " + pid : "kill " + pid;
String command = (Os.isWindows()) ? "taskkill /F /PID " + pid : "kill " + pid;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra braces around Os.isWindows()

/**
* Os detections
*/
public class Os {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about renaming Os to OS?
It seems more natural for me because "OS" is a well-known acronym.

// Modules path is prepended with a env property
if (System.getenv("MODULES") != null && System.getenv("MODULES").trim().length() > 0) {
for (String m : System.getenv("MODULES").split(System.getProperty("os.name").startsWith("Windows") ? ";" : ":")) {
for (String m : System.getenv("MODULES").split(Os.isWindows() ? ";" : ":")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use just File.pathSeparator instead of this tricky hack?
According to javadoc,

On UNIX systems, this character is ':'; on Microsoft Windows systems it is ';'.

Copy link
Member Author

Choose a reason for hiding this comment

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

Create another issue #1121

… to the same one

* refactor(os): add a new utils class to make easiest the os detection
@xael-fry xael-fry force-pushed the 2079_fileComparaison branch from f774bf0 to f628291 Compare March 15, 2017 00:47
@xael-fry xael-fry merged commit a95a35f into playframework:master Mar 28, 2017
@xael-fry xael-fry deleted the 2079_fileComparaison branch March 28, 2017 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants