Skip to content

Conversation

@JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Dec 8, 2020

Copy link
Contributor

@kl0tl kl0tl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn’t we also upgrade the bower dependencies to master?

@JordanMartinez
Copy link
Contributor Author

Shouldn’t we also upgrade the bower dependencies to master?

Hm... I guess it doesn't hurt. Who knows. It might be required for those who will still decide to use bower when they update their library?

@thomashoneyman
Copy link
Contributor

Shouldn’t we also upgrade the bower dependencies to master?

Yea, we should upgrade the bower dependencies to master for all of these -web updates. Fortunately, it looks like that's been done on almost all the other PRs already.

@JordanMartinez
Copy link
Contributor Author

Just waiting for CI now.

@hdgarrood
Copy link

I think the same reasoning for only including bower.json in the core libraries applies here: I don't think we gain much from having both bower and spago configurations in the repo. It just means there's two workflows we need to maintain rather than one. We shouldn't keep two sets of dependencies in the repo if we're not maintaining them both, so if we want to maintain both bower and spago configurations here then we should ensure both of them are tested by CI. However, you don't need to have a spago.dhall in a package's repo in order to consume that package with spago, so from my point of view I think it's preferable to only support bower for people working on the package.

@hdgarrood
Copy link

Let's have this discussion on purescript/purescript#3942

@JordanMartinez
Copy link
Contributor Author

Sounds good. I'll wait for you to post there.

@JordanMartinez JordanMartinez deleted the updateTo14 branch December 10, 2020 02:33
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.

4 participants