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

enable edit on github button #94

Closed
wants to merge 1 commit into from
Closed

Conversation

sgbaird
Copy link
Contributor

@sgbaird sgbaird commented Jan 22, 2024

@CLAassistant
Copy link

CLAassistant commented Jan 22, 2024

CLA assistant check
All committers have signed the CLA.

@@ -215,6 +215,9 @@
# Logos. Location is relative to _static folder.
"light_logo": "logo1.svg", # Logo for light mode
"dark_logo": "logo1.svg", # Logo for dark mode
"source_repository": "https://github.com/emdgroup/baybe/",
"source_branch": "main",
"source_directory": "docs/",
}

# Ignored links for linkcheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add this change to CHANGELOG

@@ -215,6 +215,9 @@
# Logos. Location is relative to _static folder.
"light_logo": "logo1.svg", # Logo for light mode
"dark_logo": "logo1.svg", # Logo for dark mode
"source_repository": "https://github.com/emdgroup/baybe/",
"source_branch": "main",
"source_directory": "docs/",
}

# Ignored links for linkcheck
Copy link
Collaborator

@Scienfitz Scienfitz Jan 23, 2024

Choose a reason for hiding this comment

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

tried it locally, this does not seem to work for autodocumentation and examples eg:
image

Reason is likely because these are converted from .py files and are not directly written as .md

@AdrianSosic @AVHopp do we care about that? It only works for user guides but overall I think its a good suggestion

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we can fix this such that it works for more than just the files that are available as .md files, then I do not see why we should not include this button. However, I also get 404 when building this locally. We would first need to investigate whether this button works as expected when being deployed on github pages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @sgbaird, thanks a lot for the suggestion, I think having the button is a very good idea. But as long as we get the 404 errors for most pages, I think we cannot include it. Do you have any clue if this is configurable, i.e. can we restrict the appearance of the button to certain pages? If not, I would perhaps open an issue on the furo repo and ask for help there...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for testing this out, I probably should have done a local build first. I will explore a bit more, but no worries if this one doesn't go through. I wasn't expecting the 404 errors 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

The best way to test the documentation would be to not only build it locally, but also build it as the github pages for your fork. This should ensure that 404 errors and stuff like that do not just happen because the local structure of the converted files is different to the way they are organized on github 😄

@Scienfitz Scienfitz added the documentation Improvements or additions to documentation label Jan 23, 2024
@Scienfitz Scienfitz added wontfix This will not be worked on and removed wontfix This will not be worked on labels Feb 1, 2024
@AVHopp
Copy link
Collaborator

AVHopp commented Feb 13, 2024

Hello @sgbaird I just wanted to ask if there are any updates on this PR resp. what the state here is :)

@sgbaird
Copy link
Contributor Author

sgbaird commented Feb 13, 2024

Hello @sgbaird I just wanted to ask if there are any updates on this PR resp. what the state here is :)

@AVHopp I've started my troubleshooting. I have a mirror GitHub Pages site running, and I'm testing out a few other things. Thank you for the ping/reminder! Will try to have either a fix or a decision to drop by the end of the week 👍

@AVHopp
Copy link
Collaborator

AVHopp commented Feb 26, 2024

Hello @sgbaird were your tests successful?

@sgbaird
Copy link
Contributor Author

sgbaird commented Feb 26, 2024

Unfortunately, no. I may come back and try sometime later. We can close for now.

@sgbaird sgbaird closed this Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants