-
-
Notifications
You must be signed in to change notification settings - Fork 167
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
Create directory structure for metric queries #65
Conversation
…rchive.org into add-contributors Update add-contributors branch
Co-Authored-By: Rick Viscomi <rviscomi@users.noreply.github.com>
Co-Authored-By: Rick Viscomi <rviscomi@users.noreply.github.com>
Co-Authored-By: Rick Viscomi <rviscomi@users.noreply.github.com>
Co-Authored-By: Rick Viscomi <rviscomi@users.noreply.github.com>
…c.httparchive.org into add-contributors
Co-Authored-By: Rick Viscomi <rviscomi@users.noreply.github.com>
…c.httparchive.org into add-contributors
…ach chapter. Progress on HTTPArchive#62
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.
Thanks Kari! Just one request to remove the Contributors changes, then this is good to go.
src/config/contributors.json
Outdated
@@ -0,0 +1,52 @@ | |||
{ |
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.
This PR should be limited to the SQL directories only. Changes related to the contributors PR should be removed.
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.
Oh, yes. I was having some issues switching between branches, and I think I did some stuff in the wrong order. I think I figured out what I need to do for the future. As soon as I figure out how to remove the contributors stuff from this branch I will do so!
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.
You should be able to straight up delete the files from within this branch and make a new commit that logs those deletions. They'll still exist in the other branch.
I think what might have happened was that you branched from that branch, which included all of its changes. Tip: I always sync/branch from master to ensure a clean branch.
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.
contributors.json
is still showing up in the PR. Can you delete it? Then this should be good to go.
It's also good practice to include a short note in the top comment about the issue this PR is addressing. For example, in this case |
src/config/contributors.json
Outdated
@@ -0,0 +1,52 @@ | |||
{ |
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.
contributors.json
is still showing up in the PR. Can you delete it? Then this should be good to go.
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.
👍
Congrats @KJLarson on merging your first PR to the Almanac! 🎉
Progress on #62