Skip to content

Conversation

JordanMartinez
Copy link
Contributor

Description of the change

Adds the flowchart I created in my learning repo's Spago-Explained.md file.

PR checklist:

  • Added the change to the "Unreleased" section of the changelog

Ok @f-f Let's discuss!

@f-f
Copy link
Member

f-f commented May 26, 2019

@JordanMartinez thanks!

As I mentioned in Slack, I think we should adjust the diagram a bit, because:

  • The global cache is coming with Add global packages cache and allow commit hashes #188 (waiting on Port tests from Python to Haskell #212 before merging, but it's otherwise ready), and I think we'd need to include it here
  • I have the feeling the diagram is trying to cover two otherwise separate aspects:
    1. what the user does or wants to do (additional concern here: should we cover all the complete tree of possibilities or just the main user flows?)
    2. what happens under the hood
      I don't have a preference on how to solve this, as it's a tradeoff: if we have a clear separation but keep everything in the same diagram then it might be comprehensive but overwhelming, while if we split into two diagrams then we might need a third one to tie the other two together. Makes sense?

@JordanMartinez
Copy link
Contributor Author

The global cache is coming with #188 (waiting on #212 before merging, but it's otherwise ready), and I think we'd need to include it here

Agreed.

I have the feeling the diagram is trying to cover two otherwise separate aspects:

I think the 'what happens under the hood' is the basic idea I'm trying to explain here. It helps one understand the larger context and why Spago uses the terms it does.

We could cover the typical user workflow in a separate flowchart or just via a step-by-step process similar to what I did in https://github.com/JordanMartinez/purescript-jordans-reference/blob/latestRelease/02-Build-Tools/05-Spago--Project-Workflow.md
That's overall easier to update than a flowchart and it follows the general flow a new user would take to create and use a Spago-based project

@JordanMartinez
Copy link
Contributor Author

@f-f Can you ping me on this PR once all dependent work is finished? Then I can get back to it.

@f-f
Copy link
Member

f-f commented May 28, 2019

Can you ping me on this PR once all dependent work is finished? Then I can get back to it.

@JordanMartinez sorry for delay, the global cache thing was kind of a big brick but now everything has been merged 🎉
I also added an explanation about the inner workings of the global cache which might be useful for the diagram

I think the 'what happens under the hood' is the basic idea I'm trying to explain here. It helps one understand the larger context and why Spago uses the terms it does.

Wonderful! Then how about splitting this in two sections under Explanations:

  • one called "What happens when you do spago build", that contains a diagram of all the steps (we can use spago build if we want to cover install+build, or spago bundle-module if we want to cover install+build+bundle)
  • the other called "Spago's Glossary" that explains all the terms that might be unfamiliar to the reader

We could cover the typical user workflow in a separate flowchart or just via a step-by-step process similar to what I did in https://github.com/JordanMartinez/purescript-jordans-reference/blob/latestRelease/02-Build-Tools/05-Spago--Project-Workflow.md
That's overall easier to update than a flowchart and it follows the general flow a new user would take to create and use a Spago-based project

Nice, this is basically what we already have in the "Super quick tutorial", except you also have a section about Parcel, which we could add here!

@JordanMartinez
Copy link
Contributor Author

@f-f I'm going to check out the latest Spago release and then get back to this. However, it's going to be a few more days before I do anything.

@JordanMartinez
Copy link
Contributor Author

I thought I'd have more time today (but that doesn't seem to be the case), so looks like this will need to be pushed back again.

I have looked at how I could do the flowchart and below was my first draft at just understanding the global cache's workflow. It's a "static" image that shows where code comes from:
static-dependencies-source

@f-f
Copy link
Member

f-f commented Jun 2, 2019

@JordanMartinez that's basically it! Some small notes:

  • the code in the global cache is not git cloned, but we just fetch the tarball. This saves a lot of space and time
  • the dependencies that are found in the global cached are not "pointed to", but the files get copied (I guess we could just point to them, but this is how it works right now)
  • the arrows are a bit confusing, as they mean "download" in the left part and "compile" in the right part

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Jun 3, 2019

the code in the global cache is not git cloned, but we just fetch the tarball. This saves a lot of space and time

Does this mean code can only be fetched from GitHub (as opposed to things like BitBucket/Gitlab)?

the dependencies that are found in the global cached are not "pointed to", but the files get copied (I guess we could just point to them, but this is how it works right now)

I think copying them makes more sense. If I deleted the global cache file, shouldn't my code still compile? If we just pointed there, that would no longer be the case. Although this is also true with local dependencies, it's fine in that situation because the end-user is the one adding them and is thus responsible for maintaining them.

the arrows are a bit confusing, as they mean "download" in the left part and "compile" in the right part

Yeah. This isn't a first draft to what I want to produce, but more just a way for me to ensure I'm understanding it correctly myself.

@f-f
Copy link
Member

f-f commented Jun 3, 2019

Does this mean code can only be fetched from GitHub (as opposed to things like BitBucket/Gitlab)?

@JordanMartinez yes, we literally hardcode GitHub here:
https://github.com/spacchetti/spago/blob/b7440b5f3673c87f1de1096d0d0d573dc050f9c2/app/Spago/GlobalCache.hs#L53-L68

Though I know that GitLab also has a url for tarballs, so if we ever get non-GitHub packages in the package set we can probably extend this.
Right now the non-GitHub packages are just cloned normally, so it's not a problem - this is just an optimization to save space and download time, as git operations will be super slow especially on big repos

@JordanMartinez
Copy link
Contributor Author

I updated my original flowchart to below. It doesn't quite follow what we've described above, but it's easier to delete things from here.

spago-flowchart

@f-f
Copy link
Member

f-f commented Jun 7, 2019

@JordanMartinez looks good. Some further comments:

  • The arrows are a bit confusing to me: the whole diagram has a "geographical" feeling to it - i.e. it shows the locations of things neatly classified into boxes - so one would expect that the arrows would mean "geographical transfer". And that's indeed the case for many steps (1, 4, 5), but not for others, where the arrows become transitions in a flowchart (2, 3?)
  • I think it would be useful to distinguish between "operations that the user does" (2a, 2b) and "operations that spago does" (everything else). Since the only outlier here is 2 we could move it from being a "step" in the process to just being a "note for users"? This would probably also solve the point above
  • In 4d there's no "linking" happening, the filepath of the current location is just passed to purs which reads them from their original location. That particular wording might confuse readers and make them think there's actual symlinking going on
  • 4b and 4c should not be independent but chained: 4b should go from the global cache to the local cache, not from the remote. This is because if a cacheable dependency is not globally cached then it will be, and at that point it just becomes a "Remote but already cached dependency" (that is, I think the "Remote but cacheable dependency" should not appear)

@JordanMartinez
Copy link
Contributor Author

Sorry for my lack of response. I was at LambdaConf this past week/weekend.

The arrows are a bit confusing to me: the whole diagram has a "geographical" feeling to it - i.e. it shows the locations of things neatly classified into boxes - so one would expect that the arrows would mean "geographical transfer". And that's indeed the case for many steps (1, 4, 5), but not for others, where the arrows become transitions in a flowchart (2, 3?)

I think it would be useful to distinguish between "operations that the user does" (2a, 2b) and "operations that spago does" (everything else). Since the only outlier here is 2 we could move it from being a "step" in the process to just being a "note for users"? This would probably also solve the point above

Perhaps I should visually distinguish one group of arrows from another? In such a case, black arrows could represent copying from one place to another, and blue arrows could represent transitions (or some other color scheme like that).

In 4d there's no "linking" happening, the filepath of the current location is just passed to purs which reads them from their original location. That particular wording might confuse readers and make them think there's actual symlinking going on

Ah... Good to know.

4b and 4c should not be independent but chained: 4b should go from the global cache to the local cache, not from the remote. This is because if a cacheable dependency is not globally cached then it will be, and at that point it just becomes a "Remote but already cached dependency" (that is, I think the "Remote but cacheable dependency" should not appear)

Hmm... Ok. I'll have to rethink how to do that one.

I'll try to finish this PR by tomorrow.

@f-f
Copy link
Member

f-f commented Jun 12, 2019

@JordanMartinez no worries, hope you enjoyed the conf! 🙂

Perhaps I should visually distinguish one group of arrows from another? In such a case, black arrows could represent copying from one place to another, and blue arrows could represent transitions (or some other color scheme like that).

I thought about that, but maybe it's better to just let go of the arrows. After all if you need to modify your package set the flow is straight: you add the packages there and eventually verify. So would it be possible to add an "info box" (maybe joined by a dotted line to the relevant step) with something like

Here you might need to further configure the package set, by:

  • adding packages or
  • override existing ones

If so, then..

Another thing I noticed is that there should be some instructions on how to edit and regenerate the diagram. We already have a section with instructions on how to hack on Spago so they could be added there

@JordanMartinez
Copy link
Contributor Author

How's this?
spago-flowchart

@JordanMartinez
Copy link
Contributor Author

nother thing I noticed is that there should be some instructions on how to edit and regenerate the diagram. We already have a section with instructions on how to hack on Spago so they could be added there

Good point. I'll work on that, too.

@JordanMartinez
Copy link
Contributor Author

A few other questions I have...

Where should I store the .graphml and exported image files? Should that be stored in the root directory or in its own diagrams or flowcharts directory?

@JordanMartinez
Copy link
Contributor Author

Sigh... That whitespace issue again... Let me fix it.

@f-f
Copy link
Member

f-f commented Jun 16, 2019

@JordanMartinez looks great!

Where should I store the .graphml and exported image files? Should that be stored in the root directory or in its own diagrams or flowcharts directory?

Let's go with diagrams

Sigh... That whitespace issue again... Let me fix it.

I actually gave up on that markdown feature, so don't worry about whitespace refactoring 🙂

@f-f
Copy link
Member

f-f commented Jun 18, 2019

@JordanMartinez thank you very much for doing this! 👏

@f-f f-f merged commit 19e3725 into purescript:master Jun 18, 2019
@JordanMartinez
Copy link
Contributor Author

Np! Thanks for working on this project. It's been very useful!

@JordanMartinez JordanMartinez deleted the addSpagoFlowchart branch June 19, 2019 01:07
elliotdavies pushed a commit to elliotdavies/spago that referenced this pull request 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

Successfully merging this pull request may close these issues.

2 participants