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

Add a reference to how it works for simple PRs #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joneshf
Copy link
Contributor

@joneshf joneshf commented Aug 6, 2022

This can be helpful to understand what's going on under the hood.

I was trying to explain how spr works to a coworker and it got kind of
confusing. I went looking for an explanation, but didn't see one. So, I
decided to throw this together in case it would be helpful for anyone
else.

This is based on my understanding of what happens from both using spr
and from reading the code. This might not be the most accurate thing, so
any improvements are welcome.

This uses the
Gitgraph diagram of
Mermaid.js to show what happens with git. This should render on
GitHub, since they
added support for Mermaid.js not too long ago.

Since the spr website uses mdBook, it should be possible to use the
mdbook-mermaid preprocessor
to render these diagrams as well. However, I haven't tested this yet
personally.

If there's a want for this kind of documentation, explaining how the
stacked workflow works will probably be even more useful. It's a bit
more complex, so understanding that can be very helpful to know what's
happening. I didn't add that right away, since it's a non-trivial amount
of work so I wanted to make sure this kind of documentation is even
wanted first.

Test Plan:
This is documentation, so aside from it making sense and being wanted
the test is whether or not it renders properly.

Created using spr 1.3.5-beta.1
@sven-of-cord
Copy link
Contributor

This is good stuff. Sorry, I haven't had little time lately for spr. I'm about to go on vacation for a week and a half. Will be back in the office in September, and reserve some time for spr.
On this PR, we should make it so that it renders correct in the book. I had a quick go at it, and it worked nicely just by installing mdbook-mermaid and adding it to book.toml. The thing I haven't had time to look at is what a good way is to get that extension in the GitHub workflow. Currently I'm using an action that installs a binary build of mdbook from somewhere, but not sure if we can get that for mdbook-mermaid. Maybe we have to install Rust and cargo install...

sven-of-cord added a commit that referenced this pull request Sep 22, 2022
This is needed for #126, which adds a new page that uses mermaid diagrams.

I reworked the book workflow a little, now using the GitHub cache action to speed things up.

Test Plan:
after submitting this PR, I will update it with temporary modifications to GitHub workflow and docs to test this. If all is well, I remove those temporary changes again

Reviewers: jozef-mokry, flooey

Reviewed By: flooey

Pull Request: #141
@adamdicarlo0
Copy link

Super helpful for me, checking out the repo and thinking about trying spr.

It seems like the workflow ends up being similar to using Gerrit? The simple case for gerrit looks identical, client-side. The only difference, I think, is that Gerrit's git server understands that an amended commit is actually version 2 of a change set. (But only because of a special identifier inserted into the commit message -- all changes get pushed to the same remote ref with Gerrit. Really a virtual ref, I guess.)

Copy link
Contributor

@jwatzman jwatzman left a comment

Choose a reason for hiding this comment

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

Thanks again for doing this. It looks great to me, with one issue inline.

The transient branch is not something you really need to directly interact with;
`spr` takes care of keeping it up to date, creating the correct commits, etc.
All you need to do is continue working on the `main` branch.
Once the PR has been created for the `spr/username/commit-title-of-C1` branch,
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you forgot part of a sentence here?

clarityflowers pushed a commit to clarityflowers/spr that referenced this pull request Mar 19, 2024
This is needed for getcord#126, which adds a new page that uses mermaid diagrams.

I reworked the book workflow a little, now using the GitHub cache action to speed things up.

Test Plan:
after submitting this PR, I will update it with temporary modifications to GitHub workflow and docs to test this. If all is well, I remove those temporary changes again

Reviewers: jozef-mokry, flooey

Reviewed By: flooey

Pull Request: getcord#141
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

4 participants