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

Fixed the broken Docker Installation Link #548

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

Lawful2002
Copy link
Contributor

fixes #547
Fixed the broken Docker Installation link in the README.rst file under the 'Setting up VulnerableCode' section

@Hritik14
Copy link
Collaborator

Hritik14 commented Sep 1, 2021

Thanks for spotting this. Before anything, please add DCO to the commit. You can do it by git commit --amend -s (make sure you agree to the DCO)
Also, I think this would still be breaking RTD @ https://vulnerablecode.readthedocs.io because of
https://github.com/nexB/vulnerablecode/blame/main/docs/source/index.rst#L23
We should entirely separate the RTD Readme file from the GitHub project readme.

@AyanSinhaMahapatra
Copy link
Member

AyanSinhaMahapatra commented Sep 1, 2021

We should entirely separate the RTD Readme file from the GitHub project readme.

Maybe it's better to have full RTD links instead of having file links?
My bad though, I should have checked the links when importing the file in RTD.

I.e. here link directly to: https://vulnerablecode.readthedocs.io/en/latest/getting-started/docker_installation.html

@Lawful2002
Copy link
Contributor Author

@Hritik14, So instead of linking the file, should I link https://vulnerablecode.readthedocs.io/en/latest/getting-started/docker_installation.html as suggested by @AyanSinhaMahapatra

@Hritik14
Copy link
Collaborator

Hritik14 commented Sep 1, 2021

We could to do that for now but hard coding a url doesn’t feel right to me (for eg: it’s pinned on the latest version via the url)

I’m writing a fresh documentation anyway so absolute url wouldn’t be a problem for the time being.

@Lawful2002
Copy link
Contributor Author

@Hritik14 I have made the changes (including the DCO), can you review them?

@Hritik14
Copy link
Collaborator

Hritik14 commented Sep 2, 2021

LGTM, thanks for the contribution.

@Hritik14 Hritik14 merged commit 2519f65 into aboutcode-org:main Sep 2, 2021
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.

Broken 'Docker Installation' link in the README file.
3 participants