chore: Add WDA-related XCUITestOptions by mykola-mokhnach · Pull Request #1552 · appium/java-client

Comment on lines +52 to +53

Optional.ofNullable(args).ifPresent((v) -> result.put("args", v));
Optional.ofNullable(env).ifPresent((v) -> result.put("env", v));

Choose a reason for hiding this comment

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

minor: parenthesis could be removed

Optional.ofNullable(args).ifPresent((v) -> result.put("args", v));
Optional.ofNullable(env).ifPresent((v) -> result.put("env", v));
Optional.ofNullable(args).ifPresent(v -> result.put("args", v));
Optional.ofNullable(env).ifPresent(v -> result.put("env", v));

Choose a reason for hiding this comment

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

I prefer to keep them for better readability

Comment on lines +64 to +70

.map((v) -> {
try {
return (v instanceof URL) ? (URL) v : new URL(String.valueOf(v));
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});

Choose a reason for hiding this comment

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

minor:

.map((v) -> {
try {
return (v instanceof URL) ? (URL) v : new URL(String.valueOf(v));
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});
.map(v -> {
try {
return v instanceof URL ? (URL) v : new URL(String.valueOf(v));
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});

Choose a reason for hiding this comment

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

see above

Comment on lines +60 to +66

.map((v) -> {
try {
return (v instanceof URL) ? (URL) v : new URL(String.valueOf(v));
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});

Choose a reason for hiding this comment

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

minor:

.map((v) -> {
try {
return (v instanceof URL) ? (URL) v : new URL(String.valueOf(v));
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});
.map(v -> {
try {
return v instanceof URL ? (URL) v : new URL(String.valueOf(v));
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});
} catch (MalformedURLException e) {
throw new IllegalArgumentException(e);
}
});

Choose a reason for hiding this comment

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

does it make sense to move the repeated logic to a new method CapabilityHelpers#toUrl(Object) ?

Choose a reason for hiding this comment

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

yes, it does

Comment on lines +39 to +41

String signingId = cert.getXcodeSigningId() == null
? DEFAULT_XCODE_SIGNING_ID
: cert.getXcodeSigningId();

Choose a reason for hiding this comment

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

String signingId = cert.getXcodeSigningId() == null
? DEFAULT_XCODE_SIGNING_ID
: cert.getXcodeSigningId();
String signingId = Optional.ofNullable(cert.getXcodeSigningId()).orElse(DEFAULT_XCODE_SIGNING_ID);

Choose a reason for hiding this comment

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

changed