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

doc: fix arch.md content #1306

Merged
merged 1 commit into from Mar 11, 2020
Merged

doc: fix arch.md content #1306

merged 1 commit into from Mar 11, 2020

Conversation

@3mard
Copy link
Contributor

3mard commented Mar 10, 2020

The link in the docs seems to be broken

@googlebot googlebot added the cla: yes label Mar 10, 2020
@zenhack

This comment has been minimized.

Copy link
Contributor

zenhack commented Mar 10, 2020

@3mard, it's working for me: https://perkeep.org/doc/terms

@3mard

This comment has been minimized.

Copy link
Contributor Author

3mard commented Mar 10, 2020

Hi @zenhack
I was viewing the docs through github https://github.com/perkeep/perkeep/blob/master/doc/arch.md
and there the link is broken (https://github.com/perkeep/perkeep/blob/master/doc/terms)

Maybe I am not supposed to view them through github ?

@zenhack

This comment has been minimized.

Copy link
Contributor

zenhack commented Mar 10, 2020

@mpl

This comment has been minimized.

Copy link
Contributor

mpl commented Mar 11, 2020

I think the links are written for the rendered site. It's a little weird to have .md in the URL for a page that's not actually markdown (though /doc/terms.md appears to redirect to /doc/terms on the live site, so it still works at least). I'll defer to the actual maintainers (I'm just a community member) but my take would be to leave it as-is. Quoting Omer Davutoglu (2020-03-10 14:37:43)

Yeah we have silly rules in the website for when/where we use .md or not, and I can never remember why and what they are. But given that most of the links in the files in doc/ seem to be .md, I think it's supposed to be that way, so I would not touch it.

@zenhack

This comment has been minimized.

Copy link
Contributor

zenhack commented Mar 11, 2020

@mpl

This comment has been minimized.

Copy link
Contributor

mpl commented Mar 11, 2020

Quoting mpl (2020-03-11 05:37:44)
I think the links are written for the rendered site. It's a little weird to have .md in the URL for a page that's not actually markdown (though /doc/terms.md appears to redirect to /doc/terms on the live site, so it still works at least). I'll defer to the actual maintainers (I'm just a community member) but my take would be to leave it as-is. Quoting Omer Davutoglu (2020-03-10 14:37:43) Yeah we have silly rules in the website for when/where we use .md or not, and I can never remember why and what they are. But given that most of the links in the files in doc/ seem to be .md, I think it's supposed to be that way, so I would not touch it.
The patch adds a suffix, not removes one. So perhaps you do want to merge this?

Oh good point, thanks. yeah.

@mpl mpl changed the title fix arch.md content doc: fix arch.md content Mar 11, 2020
@mpl
mpl approved these changes Mar 11, 2020
@mpl mpl merged commit c2e3137 into perkeep:master Mar 11, 2020
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@3mard 3mard deleted the 3mard:fix-docs-links branch Mar 11, 2020
@willnorris

This comment has been minimized.

Copy link
Member

willnorris commented Mar 11, 2020

@mpl that silly reason is partially my fault 😕 and is related to #1146

@mpl

This comment has been minimized.

Copy link
Contributor

mpl commented Mar 11, 2020

@willnorris Ah I knew it was somewhat documented somewhere but could not remember where, sorry (and thanks). No objection to that PR then, right?

We should somehow pin that issue (#1146) somewhere.

@willnorris

This comment has been minimized.

Copy link
Member

willnorris commented Mar 11, 2020

I think it might be further documented somewhere else as well, but I'm not finding it right now. I should still go back and resolve that issue at some point!

And no, of course no objection to the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.