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 ORCID to checklist #1405

Merged
merged 5 commits into from
Mar 30, 2021
Merged

Add ORCID to checklist #1405

merged 5 commits into from
Mar 30, 2021

Conversation

keyabarve
Copy link
Contributor

Fix #674

Checklist

  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@keyabarve
Copy link
Contributor Author

Please review @ctb @luizirber

@codecov
Copy link

codecov bot commented Mar 23, 2021

Codecov Report

Merging #1405 (be6cc44) into latest (7ed6291) will increase coverage by 5.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           latest    #1405      +/-   ##
==========================================
+ Coverage   89.27%   94.49%   +5.21%     
==========================================
  Files         123       96      -27     
  Lines       18790    15176    -3614     
  Branches     1447     1447              
==========================================
- Hits        16775    14340    -2435     
+ Misses       1782      603    -1179     
  Partials      233      233              
Flag Coverage Δ
python 94.49% <ø> (ø)
rust ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/core/src/sketch/minhash.rs
src/core/src/index/storage.rs
src/core/src/sketch/nodegraph.rs
src/core/tests/test.rs
src/core/src/ffi/minhash.rs
src/core/src/index/bigsi.rs
src/core/src/ffi/signature.rs
src/core/src/ffi/mod.rs
src/core/src/errors.rs
src/core/tests/minhash.rs
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ed6291...be6cc44. Read the comment docs.

@ctb
Copy link
Contributor

ctb commented Mar 23, 2021

thanks, keya!

suggest changing language to "please note your ORCID in the comments. If you don't have one, you can register for one here."

also, suggest you note your ORCID in the comments 😁

@keyabarve
Copy link
Contributor Author

@ctb @luizirber
I made the necessary changes. Please review.

@ctb
Copy link
Contributor

ctb commented Mar 27, 2021

hi @keyabarve thanks! Change requests:

  • first, the link in the markdown doesn't render. You need to move the period after ] to the end of the sentence in this text [here].(https://orcid.org/register)
  • second, what's your ORCID? please register for one and when you get it, put it in the comments here :)

thanks!

@ctb
Copy link
Contributor

ctb commented Mar 27, 2021

oh, and after you make the requested change, please update your branch from latest so that it includes the latest changes on our main branch (you can use the Update branch button, or you can do git pull origin latest; git push on the command line).

@keyabarve
Copy link
Contributor Author

The link to my ORCID is: https://orcid.org/0000-0003-3241-2117
I will make the necessary changes and commit them. I'm not sure about how to update the branch. Could you provide some more clarification?

@ctb
Copy link
Contributor

ctb commented Mar 28, 2021

hi @keyabarve, there is an "Update branch" button on this pull request; scroll to the bottom, look to the right. If you use that, you need to also do git pull origin when git status shows that you are on your ORCIDchecklist branch.

That will do the equivalent of:

git pull origin latest
git push origin ORCIDchecklist

on your working computer.

@keyabarve
Copy link
Contributor Author

On the command line of my computer, I did:
git pull origin latest
git push origin ORCIDchecklist
Please review.

@ctb
Copy link
Contributor

ctb commented Mar 29, 2021

it is updated, yay!

however, the markdown issue mentioned above still needs a fix.

thanks!

@keyabarve
Copy link
Contributor Author

I have fixed the issue!
Please review.
Thanks!

@ctb ctb merged commit f4a6918 into latest Mar 30, 2021
@ctb ctb deleted the ORCIDchecklist branch March 30, 2021 02:41
@ctb
Copy link
Contributor

ctb commented Mar 30, 2021

thanks! merging now!

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.

Add ORCID to checklist
2 participants