O3-3512: SDK should handle reading dependencies from content.properties and ensuring they are part of the distro #280

mherman22 · 2024-07-31T15:41:54Z

Description of what I changed

Issue I worked on

see https://openmrs.atlassian.net/browse/O3-3512

Checklist: I completed these to help reviewers :)

My IDE is configured to follow the code style of this project.

No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)

No? -> write tests and add them to this commit git add . && git commit --amend
I ran mvn clean install right before creating this pull request and
added all formatting changes to my commit.

No? -> execute the above command
All new and existing tests passed.

No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

ibacher

Thanks @mherman22! Great work here. A few things that need to be fixed, but a very good first pass. Also note the O in O3 is the letter "O" rather than the number 0.

ibacher · 2024-08-01T14:48:27Z

maven-plugin/pom.xml

+        <dependency>
+            <groupId>com.vdurmont</groupId>
+            <artifactId>semver4j</artifactId>
+            <version>3.1.0</version>
+        </dependency>


Could this be implemented using the java-semver library we already have instead of adding a new dependency?

True, I've just seen that, lemi use the existing library!

So, I asked this of the other PR, but could we use the org.semver:semver4j version. It's more maintained than this one.

ibacher · 2024-08-01T14:49:23Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

+	 * @param distroProperties
+	 * @throws MojoExecutionException
+	 */
+	private void parseContentProperties(File userDir, DistroProperties distroProperties) throws MojoExecutionException {


These checks should be implemented for both the build-distro and setup commands, so it might make sense to extract the "meat" of them into one of the shared components (or add a new one).

ibacher · 2024-08-01T14:51:00Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

+		for (String dependency : contentProperties.stringPropertyNames()) {
+			String versionRange = contentProperties.getProperty(dependency);
+			String distroVersion = distroProperties.get(dependency);
+
+			if (distroVersion == null) {
+				String latestVersion = findLatestMatchingVersion(dependency, versionRange);
+				if (latestVersion == null) {
+					throw new MojoExecutionException("No matching version found for dependency " + dependency);
+				}
+
+				distroProperties.add(dependency, latestVersion);
+			} else {
+				checkVersionInRange(dependency, versionRange, distroVersion);
+			}
+		}


Not all properties in the content.properties specify some sort of library. Specifically, I think it's only the keys that start: omod., owa., and spa.frontendModule. (plus, I suppose, content.

Oh! and war!

@ibacher For example, what will be the value for content.?

content.hiv=1.0.0 or something. If we have a content package that requires another content package.

(Assume the default groupId is org.openmrs.content, but can be anything using the content.hiv.groupId=xxx pattern).

ooh perfect!

ibacher · 2024-08-01T14:51:53Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

+	}
+
+	private String findLatestMatchingVersion(String dependency, String versionRange) throws MojoExecutionException {
+		return distroHelper.findLatestMatchingVersion(dependency, versionRange);


Here we need slightly different handling for findLatestMatchingVersion for keys that are spa.frontendModule. and others, since frontendModule versions need to be looked up from an NPM registry rather than a Maven registry.

ibacher · 2024-08-01T14:52:47Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

+     */
+	private void checkVersionInRange(String contentDependencyKey, String contentDependencyVersionRange, String distroPropertyVersion) throws MojoExecutionException {
+		Semver semverVersion = new Semver(distroPropertyVersion, Semver.SemverType.NPM);
+		String[] ranges = contentDependencyVersionRange.split(",");


I don't think we need to support comma-separated ranges. The default SemVer ranges have enough disjuctivity.

ibacher · 2024-08-01T14:54:04Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

+				if (range.startsWith("^") || range.startsWith("~")) {
+					inRange = semverVersion.satisfies(range);
+				} else {


I think the test here should always be something like semverVersion.satisfies(range). Ranges can also be specified as things like 1.x or 1.10.1 || ^2 || ~4.0 || > 5.

ibacher · 2024-08-01T14:54:45Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

+		}
+
+		if (!inRange) {
+			throw new MojoExecutionException(


Error message should probably also include a way of identifying the content package in question since a distro may have several.

Oh, makes perfect sense

ibacher · 2024-08-01T14:55:30Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DistroHelper.java

@@ -517,4 +518,11 @@ public Properties getArtifactProperties(Artifact artifact, Server server, String
 		return properties;
 	}

+	public String findLatestMatchingVersion(String dependency, String versionRange) throws MojoExecutionException {
+		if (dependency.startsWith("omod") || dependency.startsWith("owa") || dependency.startsWith("spa.frontend")) {


Should probably support war too, no?

ibacher

Quick remark. I haven't fully reviewed. Also, based on today's conversation, can we switch from the java-semver library to semver4j. Sorry for the whiplash!

ibacher · 2024-08-06T16:58:45Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/NpmVersionHelper.java

+
+public class NpmVersionHelper {
+
+    public String getLatestReleasedVersionFromNpmRegistry(PackageJson packageJson) {


Clever! Could I suggest just using npm dist-tag ls <package> though? Easier output to parse, etc.

That said, the overall goal here is that the user supplies some semver range and we resolve that to a version which may or may not always be the latest version. Here's what I'd suggest: use the existing NPM helper stuff to run npm pack --dry-run --json <package>@<version>, e.g., npm pack --dry-run --json '@openmrs/esm-login-app@>=5.0.0-pre.1' will get you a JSON document that has the actual resolved version, etc.

@mherman22 Any thoughts on this?

ibacher · 2024-08-08T19:39:34Z

maven-plugin/pom.xml

+        <dependency>
+            <groupId>com.vdurmont</groupId>
+            <artifactId>semver4j</artifactId>
+            <version>3.1.0</version>
+        </dependency>


So, I asked this of the other PR, but could we use the org.semver:semver4j version. It's more maintained than this one.

ibacher · 2024-08-08T19:39:48Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

@@ -78,7 +76,7 @@ public class BuildDistro extends AbstractTask {

 	private static final Logger log = LoggerFactory.getLogger(BuildDistro.class);

-	/**
+    /**


The indentation here looks funny to me.

Could you double-check that this is a tab and not spaces?

ibacher · 2024-08-08T19:43:22Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/PropertiesUtils.java

+	 *                          currently used in the distribution.
+	 * @throws MojoExecutionException if there is an error reading the content.properties file or if a version conflict is detected.
+	 */
+	public static void parseContentProperties(File userDir, DistroProperties distroProperties) throws MojoExecutionException {


I'm not sure I quite get the use-case here? The distro.properties should have entries like:

content.hiv=1.0.0-SNAPSHOT

This should correspond to a Zip file stored in a Maven repository and the content.properties file is in that Zip file. I know there are some bits missing here, but for arguments sake, it should be similar to the code to load org.openmrs:distro-emr-frontend:3.0.0:zip and read the files.

i am struggling to get what you mean here @ibacher.

Yeah... past Ian doesn't make a lot of sense. I think what I was getting at is that this assumes that the content.properties file is in the directory with all the other files, but it's likely inside a zip file inside that folder or something.

I also couldn't quite see how things would work if there is more than one content package (which is a use-case we're supporting)?

Thanks , however I thought this particular ticket was to basically pick a content.properties that has already been extracted by https://openmrs.atlassian.net/browse/O3-3640 and then do the rest of the stuff?

Cc: @ibacher

Otherwise if I'm wrong, feel free to review the latest commit 😄

ibacher · 2024-08-08T19:44:47Z

sdk-commons/pom.xml

+        <dependency>
+            <groupId>com.vdurmont</groupId>
+            <artifactId>semver4j</artifactId>
+            <version>3.1.0</version>


Only specify the version in the main pom.xml. Also, I don't think the scope is, strictly speaking necessary.

ibacher · 2024-08-08T19:54:46Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/NpmVersionHelper.java

+
+public class NpmVersionHelper {
+
+    public String getLatestReleasedVersionFromNpmRegistry(PackageJson packageJson) {


@mherman22 Any thoughts on this?

ibacher · 2024-08-20T14:14:19Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/model/DistroProperties.java

+        if (dependency.startsWith("omod") || dependency.startsWith("war")) {
+            properties.setProperty(dependency, version);
+            log.info("Added dependency: {} with version: {}", dependency, version);
+        }


This should work with spa.frontendModules and owa too, right?

ibacher · 2024-08-20T14:26:32Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/NpmVersionHelper.java

+        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+        StringBuilder outputBuilder = new StringBuilder();
+        String line;
+        while ((line = reader.readLine()) != null) {
+            outputBuilder.append(line);
+        }


More about the try() part, but reading with a single array buffer is slightly more efficient.

Suggested change

BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));

StringBuilder outputBuilder = new StringBuilder();

String line;

while ((line = reader.readLine()) != null) {

outputBuilder.append(line);

}

StringBuilder outputBuilder = new StringBuilder();

char[] buffer = new char[4096];

try (InputStream is = process.getInputStream()) {

while ((int read = is.read(buf) >= 0) {

outputBuilder.append(buf, 0, read);

}

}

thanks, though im gonna use a Reader than an InputStream. It is really a byte vs char war

ibacher · 2024-08-20T14:27:41Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/NpmVersionHelper.java

+
+        int exitCode = process.waitFor();
+        if (exitCode != 0) {
+            throw new RuntimeException("npm pack --dry-run --json command failed with exit code " + exitCode);


Probably good to also dump the contents of the output builder here.

ibacher · 2024-08-20T14:34:11Z

We may need to think about how to get the content packages Zip. I'm not entirely sure that we store the Zip file anywhere other than in the Maven repo. Maybe we use the Maven utilities to move the content packages to a temporary directory?

ibacher · 2024-08-20T17:38:42Z

sdk-commons/pom.xml

+        <dependency>
+            <groupId>org.json</groupId>
+            <artifactId>json</artifactId>
+            <version>20230618</version>
+        </dependency>


Forgot this: we actually have Jackson as a dependency already. Can we convert the code to use that instead of adding a new JSON-handling library please?

ibacher · 2024-08-22T15:19:54Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/model/DistroProperties.java

+	/**
+	 * Adds a dependency with the specified version to the properties.
+	 *
+	 * @param dependency The name of the dependency.
+	 * @param version The version of the dependency.
+	 */
+	public void add(String dependency, String version) {
+		if (StringUtils.isBlank(dependency) || StringUtils.isBlank(version)) {
+			log.error("Dependency name or version cannot be blank");
+			return;
+		}
+
+		if (dependency.startsWith("omod.") || dependency.startsWith("owa.") || dependency.startsWith("war")
+		        || dependency.startsWith("spa.frontendModule")) {
+			properties.setProperty(dependency, version);
+			log.info("Added dependency: {} with version: {}", dependency, version);
+		}
+	}
+
+	public String get(String contentDependencyKey) {
+		return properties.getProperty(contentDependencyKey);
+	}
 }


This file is mostly indented with spaces, but this change adds tabs for this section only. It's preferable to be consistent about this.

ibacher · 2024-08-22T15:20:12Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DistroHelper.java

-	}
+    }


More tabs v spaces changes.

ibacher · 2024-08-22T15:20:44Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DistroHelper.java

@@ -48,14 +50,18 @@ public class DistroHelper {

 	final VersionsHelper versionHelper;

+	NpmVersionHelper npmVersionHelper;


Does this ever get set?

ibacher · 2024-08-22T15:20:50Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DistroHelper.java

@@ -48,14 +50,18 @@ public class DistroHelper {

 	final VersionsHelper versionHelper;

+	NpmVersionHelper npmVersionHelper;
+
+	PackageJson packageJson;


Do we ever use this?

yes we do! within the same class at:

public String findLatestMatchingVersion(String dependency, String versionRange) { if (dependency.startsWith("omod") || dependency.startsWith("owa") || dependency.startsWith("content.") || dependency.startsWith("war.")) { return versionHelper.getLatestReleasedVersion(new Artifact(dependency, "latest")); } else if (dependency.startsWith("spa.frontendModule")) { packageJson.setName(dependency.substring("spa.frontendModules.".length())); return npmVersionHelper.getResolvedVersionFromNpmRegistry(packageJson, versionRange); } throw new IllegalArgumentException("Unsupported dependency type: " + dependency); }

Ah! Where does it get initialized?

mherman22 · 2024-08-23T11:15:28Z

We may need to think about how to get the content packages Zip. I'm not entirely sure that we store the Zip file anywhere other than in the Maven repo. Maybe we use the Maven utilities to move the content packages to a temporary directory?

is this the implementation that is expected? i.e we have a distro.properties file which has all the properties and their corresponding values needed to build a distro and it looks something like,

name=Ref 3.x 
distro version=3.0.0
war.openmrs=2.7.0-SNAPSHOT
omod.fhir2=1.9.0
omod.webservices.rest=2.39.0
omod.uicommons=2.23.0
omod.uiframework=3.23.0
omid.uilibrary=2.0.7
omod.idgen=4.10.0
omod.spa=1.0.6
spa.core=3.3.1
spa.frontendModules.@openmrs/esm-login-app=3.3.1 
spa.frontendModules.@openmrs/esm-patient-chart-app=3.1.0
content.hiv=1.0.0
content.pmtc=2.0.0
content.anc=2.4.5

Should be able to filter out the properties starting with content. and fetch those versions of each content property key from the maven repo, store them in a temporary directory that is created and should be deleted after the processing.

Now go into the temp directory and process/unzip the zip files in there by also making sure each zip contains at least one content.properties file.

And then we process the content.properties by reading all underlying content.properties files and check dependencies against distro.properties.

For dependencies defined in both content.properties and distro.properties, this should ensure that the version specified in distro.properties matches the range defined in content.properties; Otherwise, it outputs an error for the user to fix.

For dependencies not defined in distro.properties but defined in content.properties, the SDK should locate the latest matching version and add that to the OMODs and ESMs used by the distro.

My latest commit tries to include all the above, still looking into fetching from maven for now.

ibacher · 2024-08-23T14:38:54Z

Sounds about right. Now that I think about it, I think we can do something very similar to the [downloadDistroProperties()](https://github.com/openmrs/openmrs-sdk/blob/0285a8c64425246b647e65601d032b272c32e8c3/sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DistroHelper.java#L253C26-L253C50) stuff, right? That:

Locates a Zip file using Maven
Downloads it to a location
Extracts the distro.properties

still looking into fetching from maven for now

Both the ModuleInstaller and the DistroHelper have similar code paths that download Maven artifacts and copy them to appropriate locations.

ibacher

Nice work! A few more pointers here, but this is really shaping up!

ibacher · 2024-08-23T14:45:56Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/PropertiesUtils.java

+			String[] contentPackages = getContentPackages(distroProperties);
+
+			for (String contentPackage : contentPackages) {
+				String version = distroProperties.get(contentPackage);
+				File zipFile = fetchZipFromMavenRepo(contentPackage, version, tempDirectory);
+				if (zipFile != null) {
+					processZipFile(zipFile, distroProperties);
+				}


Wouldn't it be more efficient to just do this:

Suggested change

String[] contentPackages = getContentPackages(distroProperties);

for (String contentPackage : contentPackages) {

String version = distroProperties.get(contentPackage);

File zipFile = fetchZipFromMavenRepo(contentPackage, version, tempDirectory);

if (zipFile != null) {

processZipFile(zipFile, distroProperties);

}

for (String key : distroProperties.getAllKeys()) {

if (!key.startsWith(CONTENT_PREFIX)) {

continue;

}

Instead of basically iterating things twice? Also, it might make sense to add getContentArtifacts() to BaseSdkProperties that matches, e.g., getModuleArtifacts()?

ibacher · 2024-08-23T14:47:20Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/PropertiesUtils.java

+
+	private static File fetchZipFromMavenRepo(String contentPackage, String version, File tempDirectory) throws MojoExecutionException {
+		// Base URL for the OpenMRS JFrog repository
+		String repositoryUrl = "https://openmrs.jfrog.io/artifactory/public/";


Maybe refactor DistroHelper and ModuleInstaller into a MavenHelper to do this? We don't want to assume everything is available in the JFrog repo (there shouldn't be any references to it in the SDK; use mavenrepo.openmrs.org in preference to it)

ibacher · 2024-08-23T14:48:27Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/PropertiesUtils.java

+	private static File fetchZipFromMavenRepo(String contentPackage, String version, File tempDirectory) throws MojoExecutionException {
+		// Base URL for the OpenMRS JFrog repository
+		String repositoryUrl = "https://openmrs.jfrog.io/artifactory/public/";
+		String groupId = "org.openmrs.content";


There's a way to override groupIds that needs to be supported here too. IIRC the syntax is something like:

omod.event=1.0.0 omod.event.groupId=org.openmrs

So we'd need to support something like:

content.hiv=1.0.0 content.hiv.groupId=com.ozone-his

Code I'm thinking of is here.

mherman22

I moved stuff to DistroHelper because of the so many changes i would have to make to get rid of the NullPointerException i was encountering.

mherman22 · 2024-08-26T12:17:39Z

integration-tests/src/test/resources/integration-test/openmrs-distro.properties

@@ -13,3 +13,4 @@ spa.frontendModules.@openmrs/esm-login-app=3.3.1
 spa.frontendModules.@openmrs/esm-patient-chart-app=3.1.0
 spa.apiUrl=notopenmrs
 spa.configUrls=foo
+content.hiv=1.0.0


@ibacher i added this so as to test whether this test would pass and i ran it locally and it is passing. It is what i used to test the functionality on the setup command.

ibacher

Thanks @mherman22!

ibacher · 2024-08-28T16:00:06Z

maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

@@ -78,7 +76,7 @@ public class BuildDistro extends AbstractTask {

 	private static final Logger log = LoggerFactory.getLogger(BuildDistro.class);

-	/**
+    /**


Could you double-check that this is a tab and not spaces?

ibacher · 2024-08-28T16:03:30Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/DistroHelper.java

+	 * @param distroProperties The distro properties file.
+	 * @return The groupId to use for this dependency.
+	 */
+	private static String checkIfOverwritten(String dependencyKey, DistroProperties distroProperties) {


Is there a reason we can't use the default checkIfOverwritten()?

…es and ensuring they are part of the distro

ibacher · 2024-08-29T14:54:02Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/model/BaseSdkProperties.java

@@ -167,7 +168,7 @@ protected String getArtifactType(String key){
    }

    protected String checkIfOverwritten(String key, String param) {
-        String newKey = key + "." + param;
+        String newKey = key + "." + param; //content.hiv


Probably don't need this long-term.

ibacher · 2024-08-29T14:55:37Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/model/BaseSdkProperties.java

@@ -189,6 +190,8 @@ protected String checkIfOverwritten(String key, String param) {
                        case TYPE_DISTRO:
 	                    case TYPE_CONFIG:
                            return properties.getProperty(PROPERTY_DISTRO_GROUP_ID, Artifact.GROUP_DISTRO);
+                        case TYPE_CONTENT:
+                            return properties.getProperty(param + ".groupId",Artifact.GROUP_CONTENT);


Can we get this to a closer parallel to TYPE_OMOD? Here we should only need to handle the default value.

ibacher · 2024-08-29T14:56:35Z

sdk-commons/src/main/java/org/openmrs/maven/plugins/utility/PropertiesUtils.java

-import org.xml.sax.SAXException;
-
-public class PropertiesUtils {
+public class PropertiesUtils  {


Can we remove the changes from this file, since they are now all not substantive changes?

ibacher · 2024-08-29T14:57:50Z

It looks like the CI build is failing with a NPE here: https://github.com/openmrs/openmrs-sdk/pull/280/files#diff-c64d06786c01f8050b86ec442ebad27e09a108f0f546875e26dc55a3ce57a334R643

mherman22 · 2024-08-29T15:01:33Z

It looks like the CI build is failing with a NPE here: https://github.com/openmrs/openmrs-sdk/pull/280/files#diff-c64d06786c01f8050b86ec442ebad27e09a108f0f546875e26dc55a3ce57a334R643

Looking into it

mherman22 force-pushed the O3-3512 branch 2 times, most recently from d82557e to abb9623 Compare August 1, 2024 02:18

mherman22 marked this pull request as ready for review August 1, 2024 13:30

mherman22 requested a review from ibacher August 1, 2024 13:31

ibacher changed the title ~~03-3512: SDK should handle reading dependencies from content.properties and ensuring they are part of the distro~~ O3-3512: SDK should handle reading dependencies from content.properties and ensuring they are part of the distro Aug 1, 2024

ibacher requested changes Aug 1, 2024

View reviewed changes

mherman22 force-pushed the O3-3512 branch 3 times, most recently from d79d0f4 to 691f901 Compare August 2, 2024 16:37

mherman22 requested a review from ibacher August 2, 2024 17:58

ibacher reviewed Aug 6, 2024

View reviewed changes

mherman22 force-pushed the O3-3512 branch from 77156f2 to 629568a Compare August 7, 2024 18:35

mherman22 requested a review from ibacher August 8, 2024 07:41

ibacher requested changes Aug 13, 2024

View reviewed changes

mherman22 force-pushed the O3-3512 branch from 25f1b4f to 629568a Compare August 14, 2024 07:53

ibacher reviewed Aug 20, 2024

View reviewed changes

ibacher reviewed Aug 22, 2024

View reviewed changes

mherman22 force-pushed the O3-3512 branch from c181c06 to a5f0a12 Compare August 22, 2024 17:28

ibacher reviewed Aug 23, 2024

View reviewed changes

mherman22 force-pushed the O3-3512 branch 2 times, most recently from 00bd1ea to dc74eb4 Compare August 25, 2024 13:18

mherman22 commented Aug 26, 2024

View reviewed changes

ibacher requested changes Aug 28, 2024

View reviewed changes

mherman22 added 3 commits August 28, 2024 19:15

03-3512: SDK should handle reading dependencies from content.properti…

f08581b

…es and ensuring they are part of the distro

ensure to find the latest matching version

01070ac

fix according to code reviews

6046c07

mherman22 added 13 commits August 28, 2024 19:15

ensure frontendModule versions are looked up from an NPM registry

90305ba

fix

2689d34

extract logic into propertiesUtils file

0996390

change back to semver4j

21acc6a

fix

cc74757

fix

be9aa33

changes according to reviews

dc165f6

change to using jackson library

d83fd21

add temporary file and make changes accordingly

e3120fa

improve implementation

e5c1b6d

improve docs

db9f61a

improve implementation

b647874

move logic to distroHelper

9a60300

mherman22 force-pushed the O3-3512 branch from 61ac072 to 747e26f Compare August 29, 2024 14:04

ibacher reviewed Aug 29, 2024

View reviewed changes

try to answer reviews and make this work

4223434

mherman22 force-pushed the O3-3512 branch from baaa54c to 4223434 Compare August 29, 2024 19:53

mherman22 requested a review from ibacher August 29, 2024 20:28

Try to fix whitespaces

93eaf0b

ibacher approved these changes Sep 5, 2024

View reviewed changes

ibacher merged commit 34fc8ae into openmrs:master Sep 6, 2024
0 of 2 checks passed

mherman22 deleted the O3-3512 branch October 18, 2024 05:40


		public class NpmVersionHelper {

		public String getLatestReleasedVersionFromNpmRegistry(PackageJson packageJson) {

		@@ -48,14 +50,18 @@ public class DistroHelper {

		final VersionsHelper versionHelper;

		NpmVersionHelper npmVersionHelper;

O3-3512: SDK should handle reading dependencies from content.properties and ensuring they are part of the distro #280

O3-3512: SDK should handle reading dependencies from content.properties and ensuring they are part of the distro #280

Conversation

mherman22 commented Jul 31, 2024 • edited Loading

Description of what I changed

Issue I worked on

Checklist: I completed these to help reviewers :)

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibacher commented Aug 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mherman22 commented Aug 23, 2024

ibacher commented Aug 23, 2024

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mherman22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibacher left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibacher commented Aug 29, 2024

mherman22 commented Aug 29, 2024

mherman22 commented Jul 31, 2024 •

edited

Loading

ibacher left a comment •

edited

Loading