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

CB-14240 - Cherry pick some android fixes from master to 7.1.x #469

Merged
merged 4 commits into from
Aug 2, 2018

Conversation

jcesarmobile
Copy link
Member

Platforms affected

android

What does this PR do?

cherry pick some master fixes to 7.1.x branch

What testing has been done on this change?

Checklist

  • Reported an issue in the JIRA database
  • 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.

@janpio
Copy link
Member

janpio commented Aug 2, 2018

#450 as well maybe?

@jcesarmobile
Copy link
Member Author

I tried and git told me it was already there, checked the code and it seems that it's really there, and git log in 7.1.x branch also shows it there. Not sure why when you do a branch compare it shows it as not included.

@codecov-io
Copy link

Codecov Report

Merging #469 into 7.1.x will increase coverage by 0.21%.
The diff coverage is 69.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##            7.1.x     #469      +/-   ##
==========================================
+ Coverage   44.02%   44.24%   +0.21%     
==========================================
  Files          17       17              
  Lines        1708     1695      -13     
  Branches      316      312       -4     
==========================================
- Hits          752      750       -2     
+ Misses        956      945      -11
Impacted Files Coverage Δ
bin/templates/cordova/Api.js 49.57% <ø> (+7.69%) ⬆️
bin/templates/cordova/lib/emulator.js 48.46% <0%> (-0.19%) ⬇️
bin/templates/cordova/lib/AndroidStudio.js 100% <100%> (+5.26%) ⬆️
bin/templates/cordova/lib/pluginHandlers.js 86.7% <80%> (+0.23%) ⬆️
bin/templates/cordova/lib/AndroidProject.js 42.6% <0%> (+1.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a97fd7...d386d9c. Read the comment docs.

@janpio
Copy link
Member

janpio commented Aug 2, 2018

Oh, fun. Cherry picking is a surprising pain sometimes.

Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

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

Good changes

@raphinesse
Copy link
Contributor

#450 already was part of the last release

@brodycj
Copy link
Contributor

brodycj commented Aug 2, 2018

I would personally favor marking 7.1.2-dev before cherry-picking the changes, like I did in GH-454, apache/cordova-ios#379, and other places. I think this should be fixed in cordova-coho doc.

@janpio
Copy link
Member

janpio commented Aug 2, 2018

Isn't it that way for master already? Probably nobody thought about releases from the version branches much.

@brodycj brodycj mentioned this pull request Aug 2, 2018
2 tasks
@brodycj brodycj merged commit 750531c into apache:7.1.x Aug 2, 2018
@jcesarmobile jcesarmobile deleted the CB-14240 branch August 2, 2018 17:19
@brodycj
Copy link
Contributor

brodycj commented Aug 2, 2018

Merged by rebase now that I marked 7.1.2-dev in GH-470.

@brodycj
Copy link
Contributor

brodycj commented Aug 2, 2018

Isn't it that way for master already?

Yes.

Probably nobody thought about releases from the version branches much.

Until now, yes.

@janpio
Copy link
Member

janpio commented Aug 2, 2018

Then it probably makes sense to be fixed in coho.

@brodycj
Copy link
Contributor

brodycj commented Aug 2, 2018

I just raised https://issues.apache.org/jira/browse/CB-14249, don't know when I will get a chance to take care of this one. Further discussion should probably be there or on mailing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants