Skip to content

Conversation

f-f
Copy link
Member

@f-f f-f commented Apr 10, 2021

This refactoring PR orchestrates a few things:

  • we remove the custom module name parser that was introduced in Do not error when there are no tests #489, using the new functionality from purs graph instead
  • we now call purs graph when instantiating a BuildEnv, so that we get the graph only once at the beginning, and have it handy anytime we are building
  • the init command has its own module (we should eventually do this with all the commands)

@f-f f-f requested a review from colinwahl April 10, 2021 16:35
Copy link
Collaborator

@colinwahl colinwahl left a comment

Choose a reason for hiding this comment

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

Look great @f-f, just saw one minor improvement.

@colinwahl
Copy link
Collaborator

the init command has its own module (we should eventually do this with all the commands)

Do you also intend for all of the commands in Spago.Build to be pulled into their own modules down the line? I could help with moving things around if I know where they should go!

Co-authored-by: Colin Wahl <[email protected]>
@f-f
Copy link
Member Author

f-f commented Apr 12, 2021

Thank you for the review @colinwahl!

Do you also intend for all of the commands in Spago.Build to be pulled into their own modules down the line? I could help with moving things around if I know where they should go!

That would be great!
So far I created such modules only to break circular dependencies, but I think ideally all the toplevel commands should have their Spago.Command.* module.
Something I'm still not sure about is the granularity of the effort, e.g. if every exported function in the Spago.Build module would go to its own module, or if that would end up being confusing and we should go for bigger modules. In any case the Spago.Build module might not disappear, but remain to hold common things such as runBackend. Or something like that 🙂

@f-f f-f merged commit 456be94 into master Apr 12, 2021
@f-f f-f deleted the remove-custom-module-parser branch April 12, 2021 13:51
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.

2 participants