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

963: Bug during composer json generation #972

Merged

Conversation

Iamwade
Copy link
Collaborator

@Iamwade Iamwade commented Feb 9, 2022

Copy link
Collaborator

@bohdan-harniuk bohdan-harniuk left a comment

Choose a reason for hiding this comment

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

Hello, @Iamwade!

Please, check my suggestions.

@@ -184,16 +184,15 @@ private String getDependenciesString(final List dependenciesList) {
composerJsonFile.getText()
);
final JSONObject jsonObject = (JSONObject) obj;
Copy link
Collaborator

Choose a reason for hiding this comment

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

jsonObject can be null as well.

@@ -184,16 +184,15 @@ private String getDependenciesString(final List dependenciesList) {
composerJsonFile.getText()
);
final JSONObject jsonObject = (JSONObject) obj;
final String versionJsonElement = jsonObject.get("version").toString();
final String versionJsonElement = jsonObject.get("version") == null
? "*" : jsonObject.get("version").toString();
final String nameJsonElement = jsonObject.get("name").toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

jsonObject.get("name") can be null as well.

.toString();
}
version = versionJsonElement;
final int minorVersionSeparator = version.lastIndexOf('.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not do this if there is no version specified.

@bohdan-harniuk bohdan-harniuk self-requested a review February 10, 2022 20:08
Copy link
Collaborator

@bohdan-harniuk bohdan-harniuk left a comment

Choose a reason for hiding this comment

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

Hello, @Iamwade!

Looks better than before!
Thank you for your work here!

@eduard13, what do you think?

Regards,

Copy link
Contributor

@eduard13 eduard13 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@bohdan-harniuk bohdan-harniuk merged commit f287948 into magento:4.3.0-develop Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug during composer json generation if in dependent module version tag is not specified
3 participants