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

spago build appears to include the test directory of (local) dependencies #102

Closed
LiamGoodacre opened this issue Feb 7, 2019 · 9 comments

Comments

@LiamGoodacre
Copy link
Member

I'm getting a compilation error saying that Main has been defined multiple times and it's pointing at the test directory of my local dependencies.

@LiamGoodacre
Copy link
Member Author

Sorry, ignore this - I'm being an idiot.

@f-f
Copy link
Member

f-f commented Feb 7, 2019

No problem! 🙂

@LiamGoodacre
Copy link
Member Author

I had some existing local dependencies that I was converting to try out spago.
I ran spago init in the respective project directories to get spago.dhall and packages.dhall - but I didn't realise it also created a src/Main.purs file.

@btrepp
Copy link
Contributor

btrepp commented Feb 8, 2019

I have done this a few times too. It's no biggy, but maybe init with files shouldn't create a main.purs?.

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Feb 8, 2019 via email

@f-f
Copy link
Member

f-f commented Feb 8, 2019

How about detecting if src exists and skipping the copy if it does?

@JordanMartinez
Copy link
Contributor

Let's step back and consider the different reasons why this would be useful and annoying.

If one is migrating from psc-package to spago, then it's probable that the user already has their project structure set up correctly. They simply want to migrate to spago. Thus, adding a Main.purs file would be annoying to them.

If one is starting a new project via spago, then it would be nice if spago created all the necessary project structure: src and test folders and other spago-related files. Thus, adding a Main.purs file would be convenient and appreciated by them.

Others might want to use spago to write a library (as that can be done now via 0.6.4.0, right?) but I don't think that's a common activity yet.

So, there are a few options here:

  • address all concerns in one command, init, with reasonable defaults and provide flags to customize it as needed for special situations
  • address the migration concern in a separate command (i.e. psc-package-migrate?) from the new project/library concern via the command, init

@f-f
Copy link
Member

f-f commented Feb 9, 2019

@JordanMartinez good points 👍

A small note first: the detection "can we migrate a psc-package file" doesn't happen specifically in the init, but every time Spago needs a spago.dhall, so this is not as clean-cut as I'd like it to be (the whole "is our configuration here" probably needs some refactoring)

Also I would like to keep the CLI-surface as small as possible, and I wouldn't like to add another command that does almost the same thing as init.

So there's a "fix" up in #105, I went with checking if src and test exist, and skipping the file copy if they do, so e.g. it would prevent what happened to @LiamGoodacre and @btrepp.
If one has sources in another folders it would still not be a problem, because they are not included by default in Spago (and here I wonder if the Config should have a setting for specifying the "source paths", since it's a common use-case)

@JordanMartinez
Copy link
Contributor

Thanks for the clarification!
Yeah, keeping the CLI-surface small makes sense. I'd prefer that anyway.

f-f added a commit that referenced this issue Feb 10, 2019
elliotdavies pushed a commit to elliotdavies/spago that referenced this issue Jul 1, 2019
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

No branches or pull requests

4 participants