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

refactor: updated helloworld.html to fix a typo #1405

Merged
merged 9 commits into from Dec 7, 2020

Conversation

harry-stark
Copy link
Contributor

There was a repeated segment in helloworld.html and the document itself was crammed so I restructured it.

@harry-stark harry-stark requested a review from a team as a code owner December 7, 2020 10:50
@jina-bot jina-bot added size/S area/core This issue/PR affects the core codebase component/resource labels Dec 7, 2020
@harry-stark harry-stark changed the title refactor: updated helloworld.html to fix a typo. refactor: updated helloworld.html to fix a typo Dec 7, 2020
Copy link
Member

@maximilianwerk maximilianwerk left a comment

Choose a reason for hiding this comment

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

Hey, harry-stark. Thanks a lot for your contribution. Some minor change requests.

<h2 style="color: #1E6E73">What just happened?</h2>
<span>This is Jina's <pre style="display: inline;">hello-world</pre>,end-to-endly. It downloads Fashion-MNIST dataset and indexes 60,000 images via Jina search framework. The index is stored into multiple <i>shards</i>. We then randomly sample unseen images as <i>Queries</i>, ask Jina to retrieve relevant results. Below is Jina's retrievals, where the left-most column is query image.</span>
<br>
<span> Intrigued? Learn more about Jina and <a href="https://opensource.jina.ai">checkout our Github!</a></span>
Copy link
Member

Choose a reason for hiding this comment

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

That should point to https://get.jina.ai/ since it is where the Readme is displayed.

<div
class="about" style="max-width: 50%;padding: 10px;">
<h2 style="color: #1E6E73">What just happened?</h2>
<span>This is Jina's <pre style="display: inline;">hello-world</pre>,end-to-endly. It downloads Fashion-MNIST dataset and indexes 60,000 images via Jina search framework. The index is stored into multiple <i>shards</i>. We then randomly sample unseen images as <i>Queries</i>, ask Jina to retrieve relevant results. Below is Jina's retrievals, where the left-most column is query image.</span>
Copy link
Member

Choose a reason for hiding this comment

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

a space missing , end-to-endly.

@maximilianwerk
Copy link
Member

Hello @harry-stark, please change the commit message. Our lint complains about the full-stop in the helloworld.html.

You can amend your commit and change it!

@maximilianwerk
Copy link
Member

The commit message of the first commit still contains a full stop ..

@maximilianwerk
Copy link
Member

recheckcla

@github-actions
Copy link

github-actions bot commented Dec 7, 2020

Jina CLA check

❤️ Thank you for your pull request. It looks like this is your first contribution to an open source project maintained by Jina AI Limited. Before we can look at your pull request, we kindly ask that you all sign our Contributor License Agreement. You can sign it by commenting in format below.


I have read the CLA Document and I hereby sign the CLA


1 out of 4 committers have signed the CLA.
@harry-stark
@jina-bot
@amrit3701
@bhavsarpratik

@jina-bot jina-bot added size/L area/entrypoint This issue/PR affects the entrypoint codebase area/helper This issue/PR affects the helper functionality area/housekeeping This issue/PR is housekeeping area/testing This issue/PR affects testing and removed size/S labels Dec 7, 2020
@harry-stark
Copy link
Contributor Author

@maximilianwerk Hey max I did the changes you requested as well the commit messages are now in line with the contribution guide.

@harry-stark
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #1405 (deb05b0) into master (af36ae6) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1405      +/-   ##
==========================================
- Coverage   81.89%   81.86%   -0.04%     
==========================================
  Files         106      106              
  Lines        6970     7040      +70     
==========================================
+ Hits         5708     5763      +55     
- Misses       1262     1277      +15     
Impacted Files Coverage Δ
jina/clients/python/grpc.py 69.84% <0.00%> (-11.12%) ⬇️
jina/drivers/querylang/select.py 79.31% <0.00%> (-6.90%) ⬇️
jina/docker/hubio.py 62.32% <0.00%> (-0.84%) ⬇️
jina/logging/profile.py 55.81% <0.00%> (-0.66%) ⬇️
jina/peapods/pod.py 83.23% <0.00%> (-0.10%) ⬇️
jina/drivers/craft.py 100.00% <0.00%> (ø)
jina/types/ndarray/generic.py 100.00% <0.00%> (ø)
jina/executors/indexers/cache.py 100.00% <0.00%> (ø)
jina/executors/evaluators/text/length.py 100.00% <0.00%> (ø)
jina/executors/evaluators/rank/precision.py 100.00% <0.00%> (ø)
... and 24 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 af36ae6...53436ae. Read the comment docs.

@maximilianwerk
Copy link
Member

Hey @harry-stark I think you messed up a little bit with you last merge of master into your branch. You can try repairing this on this branch, but I believe the easiest way is: Create a new branch from master on you fork, copy the code changes onto this new branch and make one commit with the right commit message. Afterwards create a new PR with this fresh branch.

Cheers and thank you a lot for your contribution. Well appreciated.

@jina-bot jina-bot added size/S and removed size/L labels Dec 7, 2020
@harry-stark
Copy link
Contributor Author

@maximilianwerk Hey max I repaired it and this time it also passed all the tests. We can try from this last time otherwise I can open a fresh PR. I think while I was improving the commit messages, some other changes would have been pushed at the same time resulting in the previous mess. Hopefully,this time it works.

@maximilianwerk
Copy link
Member

Hey @harry-stark the result looks better now. Anyhow, the commit history will be tainted in this way, since you included several unrelated commits on the go. Please open a new PR with a fresh branch where only your commits are included.

@JoanFM
Copy link
Member

JoanFM commented Dec 7, 2020

Hey @harry-stark the result looks better now. Anyhow, the commit history will be tainted in this way, since you included several unrelated commits on the go. Please open a new PR with a fresh branch where only your commits are included.

let's maybe just squash and merge @maximilianwerk ?

@maximilianwerk maximilianwerk merged commit 4afdd2d into jina-ai:master Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core This issue/PR affects the core codebase area/entrypoint This issue/PR affects the entrypoint codebase area/helper This issue/PR affects the helper functionality area/housekeeping This issue/PR is housekeeping area/testing This issue/PR affects testing component/resource size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants