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

Remove built files from repo, therefore also remove bower support. #293

Merged
merged 2 commits into from
Jul 1, 2016

Conversation

graingert
Copy link
Collaborator

@graingert graingert commented Jul 1, 2016

Including built files in the source tree make merging multiple PRs impossible. Remove them, and therefore bower support.

Bower users must have npm installed anyway, they can install and use the package with npm from the ./node_modules directory.

@graingert graingert force-pushed the remove-bower-and-built-files branch 4 times, most recently from 9149ad8 to 32339b6 Compare July 1, 2016 10:50
Thomas Grainger added 2 commits July 1, 2016 12:11
Built files make pull requests hard to merge. It's better to generate
the files on the fly in 'prepublish'
If you're using bower use:

    npm install --save ng-infinite-scroll
    <script src='node_modules/ng-infinite-scroll/build/ng-infinite-scroll.js'></script>
@graingert graingert force-pushed the remove-bower-and-built-files branch from 9149ad8 to 921aa13 Compare July 1, 2016 11:11
@graingert graingert merged commit 5ae892a into master Jul 1, 2016
@graingert graingert deleted the remove-bower-and-built-files branch July 1, 2016 11:15
@sroze sroze removed the in progress label Jul 1, 2016
@atfornes
Copy link

atfornes commented Aug 9, 2016

thanks for the links,

Should then the info in https://www.npmjs.com/package/ng-infinite-scroll#getting-started be updated?

@graingert
Copy link
Collaborator Author

@atfornes that's because there's yet to be a release

@graingert graingert mentioned this pull request Aug 12, 2016
@gfviegas
Copy link

I dont like how the plugin makes you use NPM and NPM only. I use bower in my project to all frontend dependencies. Why would I use npm just for this plugin? People use bower, just because it's a npm dependency is not a reason to not support it. You should have a different repository to bower.

Don't wanna mix my assets folders, im turning down this plugin.

@graingert
Copy link
Collaborator Author

Bower and lodash make you use npm too. I don't like how bower makes it
difficult to merge multiple different PRs.

On 15 Aug 2016 15:39, "Gustavo Viegas" [email protected] wrote:

I dont like how the plugin makes you use NPM and NPM only. I use bower in
my project to all frontend dependencies. Why would I use npm just for this
plugin? People use bower, just because it's a npm dependency is not a
reason to not support it. You should have a different repository to bower.

Don't wanna mix my assets folders, im turning down this plugin.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#293 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZQTL1p06LpdZaRrZ64-M2VQvskwBfdks5qgHoBgaJpZM4JC_CE
.

@ulion
Copy link

ulion commented Aug 17, 2016

You can create a branch for the bower to use, or create a new repo, but please do not abandon it.

@graingert
Copy link
Collaborator Author

I don't have the time, you're welcome to do it however

On 17 Aug 2016 1:03 pm, "Ulion" [email protected] wrote:

You can create a branch for the bower to use, or create a new repo, but
please do not abandon it.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#293 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZQTAPktf1QUbm3Ah-BTbEEMvCgWXu-ks5qgviQgaJpZM4JC_CE
.

@ulion
Copy link

ulion commented Aug 17, 2016

I just tried bower install, it give me v1.3.0, it looks good so far.

2016-08-17 20:04 GMT+08:00 Thomas Grainger [email protected]:

I don't have the time, you're welcome to do it however

On 17 Aug 2016 1:03 pm, "Ulion" [email protected] wrote:

You can create a branch for the bower to use, or create a new repo, but
please do not abandon it.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#293
issuecomment-240391411>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZQTAPktf1QUbm3Ah-
BTbEEMvCgWXu-ks5qgviQgaJpZM4JC_CE>
.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#293 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKsHSWm3Wvxpaz8K10MsB9S39b3JoQLks5qgvjmgaJpZM4JC_CE
.

Ulion

@graingert
Copy link
Collaborator Author

Yeah that's the last supported version

On 17 Aug 2016 1:06 pm, "Ulion" [email protected] wrote:

I just tried bower install, it give me v1.3.0, it looks good so far.

2016-08-17 20:04 GMT+08:00 Thomas Grainger [email protected]:

I don't have the time, you're welcome to do it however

On 17 Aug 2016 1:03 pm, "Ulion" [email protected] wrote:

You can create a branch for the bower to use, or create a new repo, but
please do not abandon it.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#293
issuecomment-240391411>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZQTAPktf1QUbm3Ah-
BTbEEMvCgWXu-ks5qgviQgaJpZM4JC_CE>
.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#293
issuecomment-240391650>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/
AAKsHSWm3Wvxpaz8K10MsB9S39b3JoQLks5qgvjmgaJpZM4JC_CE>
.

Ulion


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#293 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZQTDCi-nyji5sGv2f_M7ZRnvWAa--Rks5qgvlCgaJpZM4JC_CE
.

@JrSchild
Copy link

Our deploy suddenly stopped working and we had to dig in why. Pretty soon we came to ngInfiniteScroll. Removing the build folder in a PATCH update broke everything. Obviously our a failed build process won't mess up our production server. But now we are stuck with version 1.3.0 until we can/will move away from bower.

This is exactly the reason why semver should be followed. We would like to keep retrieving automatic patches with good faith. However, stuff like this makes a lot harder for us to monitor each and every update before we can include it. Please don't make changes with major impact on a patch version.

@graingert
Copy link
Collaborator Author

@JrSchild Apologies for the down-time of your builds, but bower support was dropped quite a while ago, and you should be aiming to upgrade to npm. However, a bower version is now available at https://github.com/ng-infinite-scroll/ng-infinite-scroll-bower it's auto-built via travis. With thanks to @vikalpj

Your builds broke as a result of configuring these auto bower builds, requiring a new tag to be released.

Because bower is not supported, the semver tags relate to npm versions, not bower. Because there were no changes to the npm code only a minor version bump was required.

The npm version is tested and verified to work. YMMV on the bower builds.

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.

6 participants