Thank you Chris for your review. Please see below: > I would favor unblocking 3.2.0 by merging PR #584 and leaving PR #503 for > another release. (#503 looks like a minor improvement that not many people > are waiting for.)
Good point. I added #503, because it looked like a trivial change to me without much risk, but that might be wrong. I have no problem with leaving that open. > would be happy to give the new release a try in Dave Alden's inappbrowser > test app, cannot promise I will get very far with it. > > A couple of side points that may be a little off topic: > > In case we need to apply non-trivial fixes to an external contribution > before merging, I generally like to open a new PR that closes the original > PR. I don't really understand this. What PR are you referring to? Is it #503 where I did a rebase and added a commit? I was not sure what's the best way how how to resolve merge conflicts in this case. I tried to keep the history clear in this PR and push it because it was enabled. I understand opening a new PR is a good way to create an alternative proposal for this change. If a change is trivial is often a question of the point of view. Let me know what's the preferred way to resolve merge conflicts and unblock PRs by pushing some changes. > Dave Alden's test app seems to be great at what we need but I did not see > much documentation. I would like to give it a try sometime, may contribute > some documentation for the benefit of other IAB n00bies. I am also > wondering if we should consider bringing the IAB test app into our Cordova, > with dpa's blessing of course. This app is a good starting point for testing some basics. I also often do tests with blank Cordova apps and just the plugin added or a production app. > On Thu, Jan 2, 2020 at 3:58 PM Niklas Merz <niklasm...@apache.org> wrote: > >> Hello everyone, >> >> I would like to do my very first release with the inappbrowser plugin. >> >> I think this plugin needs a minor release soon because users are waiting >> for important patches [1] and [2] to be released. This should be a minor >> release since this version aims to support the "WKWebViewOnly" flag from >> cordova-ios@5.1.0. >> >> All PRs in this release should be tracked in the milestone for 3.2.0 >> [3]. I would appreciate any additional reviews for the two open PRs. >> They look good and ready to be merged to me. >> >> Any outstanding patches to land? >> >> If not, I would merge the outstanding PRs and start the release on >> Saturday. If somebody has any objections please let me know in this thread. >> >> This is my first Apache release and I am trying to follow the documented >> process, but please let me know if I do anything wrong or forget something. >> >> Thank you for on-boarding me and I am looking forward to do this release. >> >> Regards >> Niklas >> >> [1]https://github.com/apache/cordova-plugin-inappbrowser/pull/534 >> [2]https://github.com/apache/cordova-plugin-inappbrowser/pull/584 >> >> [3]https://github.com/apache/cordova-plugin-inappbrowser/milestone/2 >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >> For additional commands, e-mail: dev-h...@cordova.apache.org >> >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
