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

docs: Add documentation based on the youtube video #10

Merged
merged 3 commits into from
Nov 11, 2023
Merged

Conversation

joshka
Copy link
Contributor

@joshka joshka commented May 16, 2023

There's a lot of technical detail that went into creating this CI configuration, and you noted that it wasn't documented particularly well in the youtube vid covering it. I've added my understanding of what you wrote, as well as some basic instructions on how to incorporate this into an existing repo. I took the approach of what would I need to understand if looking at each action / workflow just by reading the code (rather than having to jump off into PRs and twitter messages to understand). I added a readme to the .github folder, which I think would be useful to help guide users into a pit of success using this repo.

Thanks for creating this. It's pretty useful.

.github/README.md Outdated Show resolved Hide resolved
kovyrin added a commit to kovyrin/vitess-grpc-rust that referenced this pull request Jun 16, 2023
@kovyrin
Copy link

kovyrin commented Jun 17, 2023

Noticed an issues with this PR. The README.md file in .github directory ends up overriding the main readme file used by Github web UI (the UI picks up this file instead of the one in the root of the project). I think it'd make sense to rename it into something else.

@joshka
Copy link
Contributor Author

joshka commented Jun 25, 2023

@kovyrin fixed the issue with the naming.

Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

This is great, thanks for taking the time! Left a few notes inline.

.github/workflows/check.yml Outdated Show resolved Hide resolved
.github/workflows/nostd.yml Outdated Show resolved Hide resolved
.github/workflows/scheduled.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/check.yml Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@c-git
Copy link

c-git commented Sep 4, 2023

TLDR

I want to make a PR that starts from this one and adds what I learn during testing and want to know if @joshka would be ok with that?

Long Version

@joshka Do you have the time to make the requested changes? I'm trying to integrate this repo into my workflow and want to have the headings you were adding. I was in the process of testing how these work and wanted a place to document what I find. Going to defer testing for now until this PR lands. I realistically don't know when I will be able to get back to this (just didn't want to duplicate work now nor step on anyone's toes) but if you haven't had time to do it yet before I get back to it would you mind me creating a "follow up" PR that starts from you PR and add what I learn?

@joshka joshka force-pushed the docs branch 3 times, most recently from 5277b90 to 70da1ab Compare October 17, 2023 02:48
@joshka
Copy link
Contributor Author

joshka commented Oct 17, 2023

@c-git apologies for not getting to this sooner. Just going through a bunch of PRs to clean up things now - I've addressed All @jonhoo's comments with the latest push.

@c-git
Copy link

c-git commented Oct 17, 2023

No worries, I know how life goes.

.github/workflows/nostd.yml Outdated Show resolved Hide resolved
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Looking great, thanks!

@jonhoo jonhoo merged commit bfee117 into jonhoo:main Nov 11, 2023
1 of 19 checks passed
@joshka joshka deleted the docs branch November 16, 2023 00:15
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.

4 participants