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

Remote login #298

Merged
merged 7 commits into from
Sep 2, 2020
Merged

Remote login #298

merged 7 commits into from
Sep 2, 2020

Conversation

n-orlowski
Copy link
Contributor

Add to concepts, shorten on training example, add see-also shortcode to relevant pages

Add to concepts, shorten on training example, add see-also shortcode to relevant pages
Copy link
Contributor

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

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

I've made a few comments. Also the link checker seems to be failing but I can't tell whether that's due to your changes or not.

@mrjones-plip Any tips?

content/en/apps/examples/training.md Outdated Show resolved Hide resolved
content/en/apps/examples/training.md Outdated Show resolved Hide resolved
content/en/apps/examples/training.md Outdated Show resolved Hide resolved
@mrjones-plip
Copy link
Contributor

the link checker seems to be failing but I can't tell whether that's due to your changes or not.
@mrjones-plip Any tips?

Oh, this is interesting! This is the error (for posterity):

 All Branches - Run Muffet link checker
    GOROOT: /opt/hostedtoolcache/go/1.14.7/x64
Run ./.github/scripts/muffet.sh
  ./.github/scripts/muffet.sh
  shell: /bin/bash --noprofile --norc -e -o pipefail {0}
  env:
    GOROOT: /opt/hostedtoolcache/go/1.14.7/x64
dial tcp4 127.0.0.1:1313: connect: connection refused
##[error]Process completed with exit code 1.

so that smells like GH Actions thought it started the instance via hugo but then when muffet tried to reach the server on localhost 127.0.0.1:1313 it failed. Let's hope it's ephemeral! I'll hit it with the "retry" stick, see what happens and report back.

@mrjones-plip
Copy link
Contributor

What in the world!? Indeed, I was right that hugu had failed, but you wouldn't know it by looking at the GH actions:

server hugo site OK

I happened to be watching and saw the failures in the build, which you can see when you expand the step:

server hugo NOT OK

So there's three bad links on apps/concepts/access.md, apps/examples/training.md and apps/reference/api.md:

  • [see-also] REF_NOT_FOUND: Ref "apps/examples/training/": "apps/concepts/access.md": page not found
  • [see-also] REF_NOT_FOUND: Ref "apps/concepts/access/#remote-login": "apps/examples/training.md": page not found
  • [see-also] REF_NOT_FOUND: Ref "apps/concepts/access/#remote-login": "apps/reference/api.md": page not found

If you fix those three, hugo should be able to start in GH Actions and link checker should run OK. But hrm - why isn't this being caught locally? Maybe the doc writers hugo isn't catching these broken see-also links and is building OK?!? I need to test this.

This is a bug in the GH Action - it should fail on the "All Branches - Serve Hugo site" step but isn't, so it's a false positive that it's failing on the next step, " All Branches - Run Muffet link checker". If I can reproduce, I'll file a bug to fix this.

Thanks for calling this out!

@mrjones-plip
Copy link
Contributor

mrjones-plip commented Sep 1, 2020

Ah, ok! sorry to be so chatty on this ticket, but indeed the build is failing locally. This is good! Now we just need to get GH actions to do the same thing

@n-orlowski be sure your build is working before you commit/push. This is what I'm seeing locally, let me know if it's not failing for you!

➜  cht-docs git:(remote-login-updates) ../hugo server 
Building sites … ERROR 2020/09/01 16:12:08 [see-also] REF_NOT_FOUND: Ref "apps/concepts/access/#remote-login": "apps/examples/training.md": page not found
ERROR 2020/09/01 16:12:08 [see-also] REF_NOT_FOUND: Ref "apps/examples/training/": "apps/concepts/access.md": page not found
ERROR 2020/09/01 16:12:09 [see-also] REF_NOT_FOUND: Ref "apps/concepts/access/#remote-login": "apps/reference/api.md": page not found
Built in 776 ms
Error: Error building site: logged 3 error(s)

@n-orlowski
Copy link
Contributor Author

@mrjones-plip thanks for looking into this! Seems to work now that I've removed the "See also" shortcodes..

@n-orlowski
Copy link
Contributor Author

Is the process to add the see-also shortcodes after these changes are merged? How do I avoid running into this again?

@mrjones-plip
Copy link
Contributor

@n-orlowski - sweet! To be clear, this is because, for example, apps/examples/training/ isn't a valid link in your see also: {{< see-also page="apps/examples/training/" >}}. This is because the trailing slash / denotes you're trying to load index.md in that dir (apps/examples/training/index.md). If you want to link to training.md then you drop the trailing slash and you're good to go!

{{< see-also page="apps/examples/training" >}}

So feel free to add them back in if you want (at least this one, I didn't check the other two).

Aaaaandd - when I was testing this, I saw that when I was running hugo it barfed the first time I put this intentionally bad see-also in, and then came correct when I fixed it. But then it did NOT barf when i broke it again. Was this experience such that your build was working when you committed it? That's bad!

@n-orlowski
Copy link
Contributor Author

@mrjones-plip Ahhhh I see!! Thanks for clarifying, I'll add them back. Tbh Hugo has been barfing for a while for me and I've sort of blacked out so I'm not 100% sure but I think that is what happened 😅

Copy link
Contributor

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

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

It looks like the content/en/apps/examples/training/admin-magic-link.png image is not used anywhere - can we just delete it now?

content/en/apps/examples/training.md Show resolved Hide resolved
@mrjones-plip
Copy link
Contributor

@n-orlowski - yeah, glad that was helpful! I'm also happy debug your local hugo branch for a 100% less barfing if you need any help. If hugo isn't happy locally, you'll push, wait for GH Actions to run, it will fail your build and you'll have to rinse and repeat until it succeeds to build. This is a pain!

Copy link
Contributor

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

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

Awesome!

Copy link
Contributor

@michaelkohn michaelkohn left a comment

Choose a reason for hiding this comment

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

Overall the changes look great! I'm not in love with the size of the config screen screenshot (it's huge), and we have some inconsistencies throughout the doc site re: screenshots vs. mockups as well as placement of images, but I'll address that separately.

Screen Shot 2020-09-02 at 2 10 34 PM

@n-orlowski n-orlowski merged commit b020287 into master Sep 2, 2020
@n-orlowski n-orlowski deleted the remote-login-updates branch September 2, 2020 18:20
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