-
Notifications
You must be signed in to change notification settings - Fork 43
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
moving contributing docs into subfolder, docs #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nits, but I think this is a good start to merge
@vbatts what else should we discuss to make this ready for merge? I'd like to (soonish or after others add their thoughts to what is merged) work on a fun "Introduction to OCI" animated vignette. |
bump Was away for work last week, back now and ready to start contributing to this. Anything we need to do to get it LGTMd and merged? |
I think it's about ready to merge. Looking over https://vsoch.github.io/org/ as it may appear, I think there could be more links to projects and the overview or layout of what projects exist. Also, as a convention in the image and runtime spec markdown, we put each sentence on its own line. Most markdown viewers still render adjacent lines as a paragraph, and for git change management it makes it more obvious for simple changes like typos and such. |
Okay I'm probably going to royally screw this up again with the rebasing and squashing. |
@vbatts you want me to go through and change every sentence on its own line? |
@vsoch I'll open an issue to track that later then :-) LGTM |
(not sure why pullapprove went to lunch for this issue) |
he was probably hungry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See review part 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review part2 halfway through container images getting tired :-) Finish tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
part 3 ... review of container images section complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here's the last part of my review..
ping! We are over 100 comments here - is this thoroughly reviewed yet? |
See unresolved comments. |
GitHub does really terrible for long chains like this - there is an entire set of comments that I didn't even see because they compressed them above - I only saw them looking at files and seeing something like: This process is not good enough for easily doing this, it's frustrating. There needs to be a strategy in place so that reviews don't add up like these (and comments get missed). |
@mikebrow your comments are good and very specific, mostly just changing words. I think next time it would be much faster for both of us if you just forked the repo and make the changes and did a PR to my branch. This is taking way too long to just shuffle around some letters. |
Shared doc editing.. hmm wonder what we could use. :-) |
My original thought was that I would add the skeleton (it was a lot of comment) and people would then do PRs for their detail work. I didn't think I'd be manually doing the detail work, so I guess this is my fault. |
Signed-off-by: Vanessa Sochat <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/LGTM
🍏 📗 💚 🥗 🥑 |
Yeah.. that was my thoughts.. get it in and then I (and others) could PR
the crap out of it going forward.
Guess I could have forked and PRd your repo too.. but again.. doesn't
really fix the underlying problem of too many things all at once in this
one giant commit.
…On Mon, Mar 18, 2019 at 3:28 PM Vanessasaurus ***@***.***> wrote:
My original thought was that I would add the skeleton (it was a lot of
comment) and people would then do PRs for their detail work. I didn't think
I'd be manually doing the detail work, so I guess this is my fault.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAcF1s2J4fJ6LG8pz-exxhf-TEjxZ03Cks5vX-jDgaJpZM4bVnvT>
.
|
👏
…On Mon, Mar 18, 2019 at 4:36 PM Mike Brown ***@***.***> wrote:
***@***.**** approved this pull request.
/LGTM
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAcF1iRvoIinAiGB15b3SVu-0b54ZKEcks5vX_jSgaJpZM4bVnvT>
.
|
@vsoch prolonged PRs are rough. lack of threads, hidden conversations, and falling of the radar are amplified. |
Agree. |
LGTM |
Woooot! Turn on thaaa GitHub pages! |
Signed-off-by: Vanessa Sochat [email protected]
This is PR number 2, moving all of the contributing docs into a subfolder docs, to replace #6.
This will need GitHub pages turned on, building from the "docs" folder to render at https://opencontainers.github.io/org