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

Removes edit this page link from every page #5629

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

agbilotia1998
Copy link
Contributor

@agbilotia1998 agbilotia1998 commented Dec 13, 2019

Description

The PR removes the Edit this page button from the documentation pages which redundantly redirects to a 404 page.

Screenshot_2019-12-13 Home - Gitcoin Developer Documentation

Refers/Fixes

#4943

Testing

No tests required.

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #5629 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5629      +/-   ##
==========================================
+ Coverage   30.06%   30.08%   +0.02%     
==========================================
  Files         248      248              
  Lines       21380    21380              
  Branches     3102     3102              
==========================================
+ Hits         6427     6433       +6     
+ Misses      14677    14671       -6     
  Partials      276      276
Impacted Files Coverage Δ
app/dashboard/embed.py 31.6% <0%> (+3.44%) ⬆️

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 ce6f397...46ae80a. Read the comment docs.

@molecula451
Copy link
Contributor

IMO, the button should be able to redirect to a valid link. But not to be removed somehow.

@agbilotia1998
Copy link
Contributor Author

agbilotia1998 commented Dec 13, 2019

@molecula451 the markdown files are generated from the python modules by configuring in pydocmd and the generated files are then rendered on the server. These files probably are not uploaded or updated on git repositories so it won't be possible to provide an edit link. The thing that can be done is error reporting, by which the user can open an issue on GitHub regarding the errors in the documentation, but that should not be the part of Edit this page default button provided by MkDocs. Can we do something else here?
@thelostone-mc @danlipert @octavioamu @owocki please review.

@agbilotia1998
Copy link
Contributor Author

@owocki @thelostone-mc @danlipert @octavioamu please review.

@@ -1,6 +1,6 @@
site_name: "Gitcoin Developer Documentation"
repo_name: 'gitcoinco/web'
repo_url: 'https://github.com/gitcoinco/web'
Copy link
Contributor

Choose a reason for hiding this comment

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

what does removing the repo_url do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danlipert MkDocs automatically depending upon the theme takes this parameter repo_url and creates an edit link over every page.

Copy link
Contributor

Choose a reason for hiding this comment

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

@agbilotia1998 I see - does removing the repo_url affect anything else in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danlipert seems nothing else gets effected.

Copy link
Contributor

Choose a reason for hiding this comment

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

@agbilotia1998 great - how did you test that nothing else was affected?

Copy link
Member

Choose a reason for hiding this comment

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

can't we set the edit_uri to '' and leave the repo url ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thelostone-mc some themes take the repo url and automatically checks hosts like github.com, if present, it automatically creates an edit link by appending /edit/master/docs to the repo_url for every page.

Copy link
Contributor

Choose a reason for hiding this comment

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

@agbilotia1998 would be good to keep the stars and forks info.. is there some way we can disable the edit links without removing the metrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danlipert @thelostone-mc this PR in pydocmd package fixes the problem by adding the edit_uri parameter in the mkdocs configuration NiklasRosenstein/pydoc-markdown#80
This change will be included in v2.0.6 which is not released yet, currently, we are using v2.0.4 and v2.0.5 also does not provide a fix for this. For now, maybe we can just comment the repo_url parameter stating to be used with v2.0.6 or other thing that can be done is cloning this package and adding edit_uri parameter at https://github.com/NiklasRosenstein/pydoc-markdown/blob/54c33c8845e5936a25667ebb20d10a9913793910/pydocmd/__main__.py#L75

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... looks like v2.0.5. was released over a year ago... in that case I'm fine with having the metrics removed for now since it doesn't look like v2.0.6 is coming any time soon.

@danlipert
Copy link
Contributor

@agbilotia1998 The core engineers are automatically notified when PRs are opened, so please don't tag us for reviews as it spams our inboxes - thanks!

@octavioamu octavioamu merged commit 278711a into gitcoinco:master Dec 18, 2019
@gitcoinbot
Copy link
Member

⚡️ A tip worth 0.13000 ETH (16.61 USD @ $127.78/ETH) has been granted to @agbilotia1998 for this issue from @owocki. ⚡️

Nice work @agbilotia1998! Your tip has automatically been deposited in the ETH address we have on file.

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.

6 participants