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

Move bower.json test deps into devDependencies field #142

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

JordanMartinez
Copy link
Contributor

Moves test dependencies into the proper field for the bower.json file. This unblocks the issue I discovered here: purescript-node/purescript-node-streams#54 (comment)

Deps were determined by looking at test.dhall.

@JordanMartinez JordanMartinez changed the title Move test deps into devDependencies Move bower.json test deps into devDependencies field Jul 19, 2023
Copy link
Member

@fsoikin fsoikin left a comment

Choose a reason for hiding this comment

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

The change is good, but the previous state was what Spago has generated, so it will happen again next time I try to spago bump-version.

@JordanMartinez
Copy link
Contributor Author

Ah... that's why this happened. Well, it fixes the problem for now. Migrating to Spago in the Node libraries might be the best way forward.

@fsoikin fsoikin merged commit 5f75e7c into purescript-spec:master Jul 19, 2023
1 check passed
@fsoikin
Copy link
Member

fsoikin commented Jul 19, 2023

I have two questions about this:

  1. What problem did the incorrect bower.json cause that made you notice the discrepancy?
  2. Can we drop support for Pulp now? Or is it still necessary for something?

@fsoikin
Copy link
Member

fsoikin commented Jul 19, 2023

I cut a new version v7.5.3 just now, but I had to do it manually via git tag, because running spago bump-version would have reverted the bower.json change. This is tricky.

@JordanMartinez JordanMartinez deleted the fix-bower-deps branch July 19, 2023 18:12
@JordanMartinez
Copy link
Contributor Author

What problem did the incorrect bower.json cause that made you notice the discrepancy?

I'm updating the Node libraries. In the node-streams library, we decided to integrate node-streams-aff into the library itself, breaking away from tradition that any Aff-related wrapper exist in its own repo. However, that repo uses spec to do its tests. Rather than rewriting the tests, I just used it as-is so we could get a release out. But because node-streams uses bower for its dependency management, it hit the issue linked in the opening thread.

Can we drop support for Pulp now? Or is it still necessary for something?

I'm not sure. Most people are likely on spago, but bower+pulp is what most of core is on.

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.

None yet

2 participants