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

ci(travis): add Xcode 10.2 & Xcode 11 #644

Closed
wants to merge 7 commits into from

Conversation

brodycj
Copy link
Contributor

@brodycj brodycj commented Jul 14, 2019

No description provided.

@janpio janpio changed the title ci(travis) add Node.js 12 & Xcode 10.2 ci(travis): add Node.js 12 & Xcode 10.2 Jul 14, 2019
@janpio
Copy link
Member

janpio commented Jul 14, 2019

no need to test on Xcode 10.1 any more?

@codecov-io
Copy link

codecov-io commented Jul 14, 2019

Codecov Report

Merging #644 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #644   +/-   ##
=======================================
  Coverage   74.24%   74.24%           
=======================================
  Files          11       11           
  Lines        1833     1833           
=======================================
  Hits         1361     1361           
  Misses        472      472

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 6d618c4...e3e59b8. Read the comment docs.

@brodycj
Copy link
Contributor Author

brodycj commented Jul 14, 2019

Now keeping Xcode 10.1 on Node.js 10

.travis.yml Outdated Show resolved Hide resolved
@brodycj brodycj changed the title ci(travis): add Node.js 12 & Xcode 10.2 ci(travis): add Xcode 10.2 Sep 26, 2019
@brodycj brodycj changed the title ci(travis): add Xcode 10.2 [WIP] ci(travis): add Xcode 10.2 Sep 26, 2019
@brodycj brodycj changed the title [WIP] ci(travis): add Xcode 10.2 [WIP] ci(travis): add Xcode 10.2 (...) Sep 26, 2019
@brodycj brodycj changed the title [WIP] ci(travis): add Xcode 10.2 (...) ci(travis): add Xcode 10.2 & Xcode 11 Sep 26, 2019
@brodycj brodycj requested a review from janpio September 26, 2019 20:45
@dpogue
Copy link
Member

dpogue commented Feb 14, 2020

We're testing with Xcode 11 only now, closing due to conflicts.

@dpogue dpogue closed this Feb 14, 2020
@brodycj
Copy link
Contributor Author

brodycj commented Feb 14, 2020

My work is now wasted since people continued updating without even considering this proposal. I did request quite a few reviewers on GitHub, evidently ignored. And I recall raising some other PRs that never got reviewed.

One thing is that it would have been nice to include Xcode 10.2 in Travis CI, as long as it can still keep working, like I proposed here.

@erisu
Copy link
Member

erisu commented Feb 14, 2020

@brodybits If you still want this merged in, change the target branch to 5.1.x.

If there were to be any future minors releases for the 5.x major, a new minor branch would need to be created based on 5.1.x. Same also applies with future patch releases, PRs would need to be merged into 5.1.x branch. Your changes in this PR can still be used to test those PRs that would be merged into that branch for any potential 5.x related releases.

@brodycj
Copy link
Contributor Author

brodycj commented Feb 16, 2020

Thanks @erisu, I think you are right since cordova-ios is rightfully dropping Xcode pre-11 in master (for upcoming major release). If we do apply this change to 5.x branch, I think we should try to ensure little things such as comments that I added are as consistent as possible with master. I really don't have so much extra time to pay attention to this right now. Some other thoughts for the record:

I think I did overreact. My work was not really such a huge amount, and I do strongly favor the review process we follow to keep our repos as clean as possible. My apologies.

Documentation needs to be updated as well, as I already reported in apache/cordova-docs#1056.

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.

5 participants