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

CB-13772: print version numbers correctly in "cordova requirements" #291

Merged
merged 1 commit into from Aug 10, 2018

Conversation

Projects
None yet
2 participants
@darcymeyer
Contributor

darcymeyer commented Jan 11, 2018

What does this PR do?

This PR fixes the bug where cordova requirements did not print out version numbers correctly.
Previously, it would print something like ios-deploy: installed [object Object]. Now it prints ios-deploy: installed 1.9.2.

What testing has been done on this change?

This was a one line fix so none of the automated testing was run. I manually verified that the fix prints the version numbers correctly for the Android and iOS requirements.

Checklist

  • Reported an issue in the JIRA database
    Issue reported here: https://issues.apache.org/jira/browse/CB-13772
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.
    (No additional testing applicable)
CB-13772: print version numbers correctly in cordova requirements
Fixes a bug where "cordova requirements" printed out
[object Object] in place of the version number.
@janpio

This comment has been minimized.

Contributor

janpio commented Mar 3, 2018

Hey @darcymeyer, this looks like a useful change - I also noticed this happening. Do you know why this suddenly is a problem?

@darcymeyer

This comment has been minimized.

Contributor

darcymeyer commented Mar 15, 2018

Hi @janpio, I have no idea when or why this bug was introduced, I just noticed it one day. Maybe someone changed the json structure in which the version information was returned and didn't update the cli?

@janpio

This comment has been minimized.

Contributor

janpio commented Mar 16, 2018

Ok, thanks for the update.

I am a bit careful merging this, as I am not 100% sure this is not somehow environment dependant or something :/

Anyone have any more insight here?

@janpio

janpio approved these changes Aug 10, 2018

@janpio

This comment has been minimized.

Contributor

janpio commented Aug 10, 2018

After looking at the code again, I am pretty confident this won't break anything.

@janpio janpio merged commit dca0e5b into apache:master Aug 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@brodybits brodybits referenced this pull request Sep 21, 2018

Merged

Start version 8.1.0 #333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment