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/readme banners #523

Merged
merged 7 commits into from
Jul 28, 2023
Merged

Fix/readme banners #523

merged 7 commits into from
Jul 28, 2023

Conversation

lorenzolewis
Copy link
Member

Updates the banners on the readme's to absolute paths so that downstream docs CI has nice banners

Signed-off-by: Lorenzo Lewis <lorenzo_lewis@icloud.com>
Signed-off-by: Lorenzo Lewis <lorenzo_lewis@icloud.com>
Signed-off-by: Lorenzo Lewis <lorenzo_lewis@icloud.com>
This reverts commit 27edc59.
Signed-off-by: Lorenzo Lewis <lorenzo_lewis@icloud.com>
@lorenzolewis lorenzolewis requested a review from a team as a code owner July 28, 2023 19:07
@FabianLars FabianLars merged commit 6f01bc1 into v2 Jul 28, 2023
3 checks passed
@FabianLars FabianLars deleted the fix/readme-banners branch July 28, 2023 19:29
@amrbashir
Copy link
Member

I feel like I prefer the relative path, which makes it resilient against branch name changing, merge conflicts, parent directory renaming ...etc. I feel like this is a problem that should be addressed downstream where the script that generates the docs for the README, should rewrite the relative URL to a permanent URL that reference the commit it was generated from.

@lorenzolewis
Copy link
Member Author

How do other downstreams like docs.rs and npm handle this? I can look into what it would take to match whatever they're doing. I'd just copied what's done here: https://github.com/tauri-apps/tauri/blob/4db363a03c182349f8491f46ced258d84723b11f/core/tauri/README.md?plain=1#L3 (and in other packages there)

@FabianLars
Copy link
Member

tbh I don't think any of the reasons listed really matter, at least in the short term, so imo we should focus on getting the docs running first and add this improvement to the Todo list for later :)

@amrbashir
Copy link
Member

How do other downstreams like docs.rs and npm handle this? I can look into what it would take to match whatever they're doing.

Crates.io seem to transform the relative link into an absolute URL, so ./splash.png will become https://raw.githubusercontent.com/tauri-apps/plugins-workspace/e47562f71284457ff77e4c8b6bf02fdbe19ab880/plugins/window-state/splash.png

I'd just copied what's done here: tauri-apps/tauri@4db363a/core/tauri/README.md?plain=1#L3 (and in other packages there)

Yeah, we probably gonna need to fix those as well.

tbh I don't think any of the reasons listed really matter, at least in the short term, so imo we should focus on getting the docs running first and add this improvement to the Todo list for later :)

I agree, I am not saying it is a blocker, just something to think about for the future.

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.

3 participants