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

[DEV] Ignoring dist files/folders #2617

Closed
NicolasCARPi opened this issue Jun 4, 2021 · 6 comments · Fixed by #2681
Closed

[DEV] Ignoring dist files/folders #2617

NicolasCARPi opened this issue Jun 4, 2021 · 6 comments · Fixed by #2681
Labels

Comments

@NicolasCARPi
Copy link
Collaborator

This issue concerns devs on this repo.

I believe the dist/ folder and the docs/docs/dist folders should be added to .gitignore and any grunt command to build or clean MUST not pollute the diff with unrelated changes.

This should help make PRs easier to work with because of smaller diffs, and it's also good practice to do so.

If this change is accepted, you might need to change your npm publish process and add a build step (or not).

@caseyjhol Is there a good reason to keep the built files in the repo?

If not, I can make a PR.

@caseyjhol
Copy link
Member

caseyjhol commented Nov 26, 2021

One benefit of keeping the dist directory, is it easily allows people to test the latest master code (before a published release) to see if their issue has been fixed. Maybe there's a better solution, though. I'm not sure how often users actually did that anyway in practice.

@NicolasCARPi
Copy link
Collaborator Author

is it easily allows people to test the latest master code

npm install bootstrap-select#master does it for you! ;)

It's really not a good practice to do this. Now commits/diffs can be much cleaner!

@reloxx13
Copy link

now dist is missing in composer beta installation

@NicolasCARPi
Copy link
Collaborator Author

now dist is missing in composer beta installation

IMHO a javascript library should not be installed with a PHP dependency management system such as composer. So this project shouldn't have a composer.json in the first place. There is already very few available "maintainer time" on this project and trying to maintain every single use case under the sun adds work for very little benefits.

Given that there are no real active projects depending on the composer package, I suggest removing the composer.json and packagist package entirely instead of trying to fix a flawed approach.

Unless one can show me a valid use of installing a JS lib through composer.

Note that you can workaround this issue with a build step on your side.

@reloxx13
Copy link

reloxx13 commented Jul 13, 2022

lmao

cakephp comes with composer and alot other php frameworks and its not like u r the only js project in composer.
if u decide like this ill instant stop using this lib 4ever as im not using multiple package installers in one project.

@NicolasCARPi
Copy link
Collaborator Author

lmao

...

cakephp comes with composer and alot other php frameworks

Of course it does. PHP dependencies come in composer, as they should. I'm not talking about this. I'm talking about installing this JS library through composer. Or any JS lib for that matter.

and its not like u r the only js project in composer.

No. But it's not because you can that you should. Installing JS libs through composer is weird and seldom used (this is an opinion, not ground truth). Look at bootstrap: installed with composer : 323, installed with npm : 10,888. The vast majority of devs use yarn or npm to install their js dependencies, then probably use a bundler like webpack.

if u decide like this ill instant stop using this lib 4ever as im not using multiple package installers in one project.

First, let me be very clear, nobody cares that you use or don't use this library. Being a Karen about it won't change a thing.

Second, I'm just voicing my opinion, we are discussing it. If the composer package can be fixed by adding a few lines somewhere so the dist/ folder reappears, I'll be happy to merge that. Otherwise, my point is that I don't think it is wise to dedicate time to fixing things like this when (in my opinion) it is weird to provide it through composer. I'm maintaining other JS libs and providing a composer install never crossed my mind. But I can understand a workflow where JS libs are installed through composer. After all, if it works for some, why not. It's just that it's marginal and if it means more work for the maintainers I'm raising the question of continuing support. Same with bower, I'd have killed it long ago. You might have noticed that this lib is full of unfixed bugs and unresolved issues because the main dev hasn't much time to dedicate to it. When this happens, a good approach is to reduce/remove cruft. Like support for IE browser. Or support for dead package managers (bower not composer).

Now you can propose a PR to fix that issue, or "stop using this lib 4ever".

I would also be happy to have the opinion of other devs/users reading this. Should composer be supported as an install option? Am I crazy to think that a PHP dependency packager should be used for PHP dependencies and a JS dependency manager for JS dependencies?

As a final note @reloxx13, please keep in mind that you are using something for free and are entitled to strictly nothing. So you need to be polite and write your messages with all letters, this is not a fortnite forum.

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

Successfully merging a pull request may close this issue.

3 participants