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

Fix path to product image. #47

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

domdfcoding
Copy link

No description provided.

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks for finding this; I'll make the path even simpler. I found a couple of other libraries with the same problem.

README.rst Outdated Show resolved Hide resolved
@dhalbert
Copy link
Contributor

@tekktrik There is a path problem here; I'm not quite sure what to do about it. Do you have an idea?

It looks like the build action works properly if the product image is ../docs/_static/3013-01.jpg, but then the README display on the GitHub page is a broken link.

If the ../ is removed, then the build fails.

Other libraries with similar broken links:
https://github.com/adafruit/Adafruit_CircuitPython_DS1307
https://github.com/adafruit/Adafruit_CircuitPython_HCSR04
https://github.com/adafruit/Adafruit_CircuitPython_PCF8523 (image path is wrong in a different way)

@tekktrik
Copy link
Member

tekktrik commented Apr 20, 2023

@dhalbert the documentation is built from within the docs/ folder so that's the expected working directory, hence why ../docs/ worked but docs/ doesn't. Remove docs/ from this PR allows the CI to build again.

Relevant line in the composite action: https://github.com/adafruit/workflows-circuitpython-libs/blob/3c236c4bdc63af724842b7c88f71e321512ca927/build/action.yml#L74

We could apply a patch that specifies the path as docs instead of . for the sphinx-build command I think, and then update the composite action to use the root folder as the working directory.

@dhalbert
Copy link
Contributor

We could apply a patch that specifies the path as docs instead of . for the sphinx-build command I think, and then update the composite action to use the root folder as the working directory.

Yes, this makes sense. README.rst is expecting the root to be above docs, and the build needs to expect the same thing.

@tekktrik
Copy link
Member

@kattni just pinging you so we can discuss a library patch maybe within the next couple weeks

@tekktrik
Copy link
Member

Assigning to myself so I don't forget about the patch this will require.

@tekktrik tekktrik self-assigned this Apr 27, 2023
@tekktrik
Copy link
Member

tekktrik commented Mar 21, 2024

@dhalbert - Coming back to this, is this still a patch we want to run, for building docs from the repo root folder instead of docs? It should be a relatively quick and simple one to organize. Let me know what you think, and I'll get it together and run it in the next week or so if we still want to do it.

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Apr 1, 2024

It makes sense to me to change to do the building from the root instead of inside of docs/ I'd say go ahead if it's a quick / easy change @tekktrik

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.

4 participants