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 broken faucet link #312

Merged
merged 4 commits into from
Sep 24, 2021
Merged

Conversation

vladanghene
Copy link
Contributor

@vladanghene vladanghene commented Sep 21, 2021

No description provided.

Copy link
Member

@katomm katomm left a comment

Choose a reason for hiding this comment

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

Hello,
what exactly is the issue here? The link to https://developers.cardano.org/docs/integrate-cardano/testnet-faucet seems to work fine.

@vladanghene
Copy link
Contributor Author

vladanghene commented Sep 21, 2021

yes well the href on that anchor is set to go above one dir ../ when in fact what you set as an example of a working link in your reply was from the dir docs/getting-started/ which indeed works. I tried setting it as an absolute url to the first google result of a query test ada faucet, not having had delved into the root structure of the getting started app.

it needs to skip two folders above ../../ to be a valid relative link to the faucet page. Fixed.

@rdlrt
Copy link
Collaborator

rdlrt commented Sep 21, 2021

The links are not broken. It seems you're browsing from github itself (instead of developers.cardano.org).

@vladanghene
Copy link
Contributor Author

this is where I was browsing from: https://developers.cardano.org/docs/get-started/testnets-and-devnets/

@katomm
Copy link
Member

katomm commented Sep 22, 2021

The link is not broken. I guess the confusion is because you think "testnets-and-devnets" and "testnet-faucet" are folders, but they are files. (testnets-and-devnets.md and testnet-faucet.md)

That means going from:
/docs/get-started/testnets-and-devnets.md

To:
/docs/integrate-cardano/testnet-faucet.md

needs only one folder up.

@katomm katomm closed this Sep 22, 2021
@vladanghene
Copy link
Contributor Author

vladanghene commented Sep 22, 2021

Okay, if you call this Not Found page working then i guess it was my confusion

@katomm
Copy link
Member

katomm commented Sep 22, 2021

I'm sorry @vladanghene, I'm reopening this.

@katomm katomm reopened this Sep 22, 2021
@katomm
Copy link
Member

katomm commented Sep 22, 2021

@vladanghene is it possible to provide the generated HTML code for that link? For me it is:
Screenshot 2021-09-22 at 09 38 15

@vladanghene
Copy link
Contributor Author

vladanghene commented Sep 22, 2021

yeap. for me too href="/docs/get-started/integrate-cardano/testnet-faucet"

i tested and it should be as I stated earlier, if you want it relative to the folder structure href="/get-started/integrate-cardano/testnet-faucet"

seems like if I make it ../../ it doesn't pass the checks on your build tests.

@katomm
Copy link
Member

katomm commented Sep 22, 2021

I'm able to reproduce this is now. While it works on Safari browsers it seems that Chromium forks add the "get-started" folder for some reason.

@vladanghene
Copy link
Contributor Author

i've been using safari from the get-go and it has never worked

@katomm
Copy link
Member

katomm commented Sep 22, 2021

seems like if I make it ../../ it doesn't pass the checks on your build tests.

Yes, because that would be wrong :) the cause here is something else.

@vladanghene
Copy link
Contributor Author

the cause is having .../docs/.. in the href, it should have /getting-started/..

@katomm
Copy link
Member

katomm commented Sep 22, 2021

I wish it were that simple. The markdown is:
[Cardano testnet faucet](../integrate-cardano/testnet-faucet)

This is correct because we are in the "get-started" folder. This is also the reason why I closed your PR wrongly. Probably a redirect gets in the way here.

@vladanghene
Copy link
Contributor Author

ok so i see now, the md is wrong having the jump to a folder above with ../
inspecting other links 2 paragraphs above, I saw that the markdown you wrote simply binds /docs/whatever as if the homeurl is https://developers.cardano.org/ and not /docs/getting-started/.

let me commit it as docs/integrate-cardano/testnet-faucet/

adding relative paths to the docs/integrate-cardano/testnet-faucet/ url
@katomm
Copy link
Member

katomm commented Sep 22, 2021

While this will work I'd advise against /docs/whatever URLs because only relative links will be checked and flagged during the build of the site. (I know that there are still links with /docs/whatever, but we should avoid them.)

@vladanghene
Copy link
Contributor Author

been looking for a relative link with the structure you want ../ to successfully jump out of get-started and resume the href from docs/... and it seems the one I see in community page related to if you want to join the [Plutus Pioneers](../smart-contracts/plutus#get-started-with-the-plutus-pioneer-program). works fine ... it jumps out of get-started.
this one here does not however ...

i think you guys should rewrite the test checks so it includes base references in md as well as /docs hrefs... other than that, I have no idea why in some place it works and in others it doesn't. It's definitely not the first time this is mentioned in regards to github's md.

@rdlrt
Copy link
Collaborator

rdlrt commented Sep 22, 2021

Looks like the issue is with docusaurus itself and how it routes between pages depending on browser's SVA context.
The markdown and related tests in it's current state should work fine if the corresponding fix for docusaurus is added.

The "fix" (essentially option to forcefully add trailingSlash which causes different formation of URL) is merged on their master, and will be part of 2.0.7-beta.

@katomm
Copy link
Member

katomm commented Sep 22, 2021

Thanks for the insights @rdlrt and thanks to @vladanghene for being persistent :)

I'd suggest replacing URLs that are affected by this issue with the absolute "/docs/whatever" as an interim solution and then wait for a final release tag of docusaurus to address this.

replaced the problematic url with /docs/whatever as an interim solution just to get the link to work for now
@vladanghene
Copy link
Contributor Author

ok replaced it as interim solution, ready to be merged.

Copy link
Member

@katomm katomm left a comment

Choose a reason for hiding this comment

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

👍

@katomm katomm merged commit 8685693 into cardano-foundation:staging Sep 24, 2021
katomm added a commit that referenced this pull request Sep 24, 2021
* Add smart-contract search entry point (#287)

* Add playgrounds (#288)

* Bump axios from 0.21.1 to 0.21.4 (#285)

Bumps [axios](https://github.com/axios/axios) from 0.21.1 to 0.21.4.
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/master/CHANGELOG.md)
- [Commits](axios/axios@v0.21.1...v0.21.4)

---
updated-dependencies:
- dependency-name: axios
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fixes Cardano Explorer and Pull Request Template links (#272)

* changes the default language of a link's site to English

* fixes link to pull request template in contributing document

Co-authored-by: Tommy

* Adding alonzo genesis files (#291)

* Adding alonzo genesis files

* Update running-cardano.md

* fix typo (#305)

* fix typo

* september spotlight interview

Co-authored-by: Kevin Cislak 

* Removing the random constraint for building wallet (#297)

Building the wallet with `--constraint="random<1.2"` results in:

```
Building library for cardano-wallet-core-2021.9.9..
[59 of 95] Compiling Cardano.Wallet.DB.Sqlite.Types ( src/Cardano/Wallet/DB/Sqlite/Types.hs, /home/cardano/cardano-src/cardano-wallet/dist-newstyle/build/x86_64-linux/ghc-8.10.7/cardano-wallet-core-2021.9.9/build/Cardano/Wallet/DB/Sqlite/Types.o, /home/cardano/cardano-src/cardano-wallet/dist-newstyle/build/x86_64-linux/ghc-8.10.7/cardano-wallet-core-2021.9.9/build/Cardano/Wallet/DB/Sqlite/Types.dyn_o )

src/Cardano/Wallet/DB/Sqlite/Types.hs:122:1: error:
    Could not load module ‘System.Random.Internal’
    It is a member of the hidden package ‘random-1.2.0’.
    Perhaps you need to add ‘random’ to the build-depends in your .cabal file.
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
    |
122 | import System.Random.Internal
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...
cabal: Failed to build cardano-wallet-core-2021.9.9 (which is required by
exe:local-cluster from cardano-wallet-2021.9.9, test:integration from
cardano-wallet-2021.9.9 and others).
```

Issue here: cardano-foundation/cardano-wallet#2824
Raising this PR to discuss as I don't understand the significance of removing the constraint.

Versions:
* cardano-wallet tag v2021-09-09
* cabal-install version 3.4.0.0
* compiled using version 3.4.0.0 of the Cabal library
* The Glorious Glasgow Haskell Compilation System, version 8.10.7
* Ubuntu 20.04.3 LTS

* september spotlight article (#306)

Co-authored-by: Kevin Cislak

* Update creating-wallet-faucet.md (#307)

* Update creating-wallet-faucet.md

* Update creating-wallet-faucet.md

* Delete duplicate article (#310)

* Use `HydraBuildList` to provide latest release binaries (with fallback) (#300)

* Bump prismjs from 1.24.1 to 1.25.0 (#311)

Bumps [prismjs](https://github.com/PrismJS/prism) from 1.24.1 to 1.25.0.
- [Release notes](https://github.com/PrismJS/prism/releases)
- [Changelog](https://github.com/PrismJS/prism/blob/master/CHANGELOG.md)
- [Commits](PrismJS/prism@v1.24.1...v1.25.0)

---
updated-dependencies:
- dependency-name: prismjs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix broken faucet link (#312)

* fix broken faucet link

* Update testnets-and-devnets.md

* Update testnets-and-devnets.md

adding relative paths to the docs/integrate-cardano/testnet-faucet/ url

* Update testnets-and-devnets.md

replaced the problematic url with /docs/whatever as an interim solution just to get the link to work for now

* minor bug (#314)

added the 0x prefix, which is needed for the CLI to recognize the string as a byte array

* Spotlight fixes (#315)

* Fix spotlight article date

* Fix inconsistent tag

* Varia (#320)

* Fix Gimbalab changes

* Add two videos to technical concepts page

* Add PC fund 5 voting results

* Refresh discord invitation link

* Remove outdated note box

* Improve integrate Cardano overview

* Fix inconsistent tAda (test ada) spellings

* Start editorial style guide

* Fix inconsistent ada spellings

* Update Editorial Style Guide

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rodrigo Matos
Co-authored-by: John Imison 
Co-authored-by: cislakk 
Co-authored-by: Kevin Cislak
Co-authored-by: Javier La Banca 
Co-authored-by: Alexander Klee 
Co-authored-by: AVA 
Co-authored-by: Martin Lang
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