search

PeterStaev / nativescript-image-swipe

A NativeScript widget to easily :point_up_2: and :mag: through a list of images
Apache License 2.0
35 stars 21 forks source link

Upgrade demo apps to NativeScript 6.0.0 #47

Closed VladimirAmiorkov closed 5 years ago

VladimirAmiorkov commented 5 years ago

I notice that my previous PR was merged with failing travis build due to demo apps not migrated to NativeScript 6.0.0. While you deleted the travis build and merge and released my PR I think for the future of the plugin it is best to not completely remove the travis stage that is executed in new PRs because this can and will cause the plugin to not work in the future. I would like to hear your opinion on what should be expected from a PR to be considered "appropriate" for merge and release because merging without any kind of testing is sure to lead to issues down the road.

For now here is what this PR brings:

PeterStaev commented 5 years ago

Hey @VladimirAmiorkov , thank you for this update! Now onto the topic for demo apps being included in the CI/CD. This is a bit of trade off - it is true that having these ensure at least some basic guarantee that the plugin would be working, but also contributors MUST update the demos in order for me to be able to publish the new version on NPM. And I have rarely seen people actually doing this (you are the first one in quite a while 😉 ). If they don't update demos, the PRs might be hanging around for a while, just because there was some breaking change in the NS app workflow and not because the plugin is not working.

For now, let's say that I am OK'ish with returning those and we will see how it will playout in the future.

VladimirAmiorkov commented 5 years ago

Hi @PeterStaev ,

First of all no problem, I love contributing to useful projects. Yes that is true about the demo apps and CI, there is a trade off but one of the things any contributor would want and actually has to do is test their fix/feature before making any PRs. If they don't then it is very likely their PR is not correct, of course that may not be the case for very straight forward PR changes but in the big picture any contributor that has not ran any of the apps in a repo he is contributing to is likely to have resolves partially the issue he may had and not the actual issue. This comes from personal experience with contributions to big open source projects to which I have made such mistakes due to the project overwhelming build structure etc causing me to not be able to run their "development apps".

To me personally if the demo apps in any open source project do not start then anyone that wants to work on it has to resolve and fix them before they can actually start working on the thing they wanted to fix. Is this going to stop anyone from contributing, probably yes but such (blind) contributions will do more harm than good. Making blind fixes with zero actual application testing is never a good idea and wont lead to anything good. This is my personal opinion and to me if I see an plugin with failing tests I would like to contribute to it to make those builds green :)