-
Notifications
You must be signed in to change notification settings - Fork 3
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
Removal of hardcoded LDS jar versions #87
Conversation
fs.copyFileSync(config.sourceLdsJarFile, | ||
path.join(config.libDirPath, config.ldsJarName)) | ||
const ldsJarMatches = glob.sync(config.sourceLdsJarFile); | ||
if (ldsJarMatches.length > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be simpler to just delete all matching jars before copying a the new one? That would also obviate the need for introducing a dependency on the glob
package, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that sounds like a good idea. We can delete the directory before we run Gradle.
This is a dev dependency, so it does not affect the size of the executable; furthermore, although we could simply copy anything that is in the directory where we expect the JAR, it is not clear to me that that would be less error-prone than comparing it either to a regex or a glob. Furthermore, it seems appropriate to me to have this error-checking because a) if the build process changes, it is good to have a sanity check (kind of like an assert
statement), and b) unexpected things can happen in the user's file system that are outside the control of this script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sounds reasonable. We can keep the regex, but the -SNAPSHOT
should be replaced with a wildcard or else this won't work with releases.
@@ -75,4 +75,4 @@ export const libDirPath = path.resolve(baseDirPath, libDirName); | |||
/** Absolute path to the language and diagram server jar. */ | |||
// TODO handle version in file name | |||
export const sourceLdsJarFile = path.resolve(baseDirPath, | |||
path.join(repoName, 'org.lflang.diagram', 'build', 'libs', 'org.lflang.diagram-0.3.1-SNAPSHOT-lds.jar')); | |||
path.join(repoName, 'org.lflang.diagram', 'build', 'libs', 'org.lflang.diagram-+(?).+(?).+(?)?(-SNAPSHOT)-lds.jar')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a weird syntax for ?
wildcards, according to the glob
documentation:
?(pattern|pattern|pattern)
Matches zero or one occurrence of the patterns provided.
Fixes #86.