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

feat: use the new api proxy and improve embed shortcode #1159

Merged
merged 6 commits into from
Sep 24, 2020

Conversation

hugomrdias
Copy link
Contributor

@hugomrdias hugomrdias commented Sep 17, 2020

This PR adds:

  • Support for the new API proxy
    • The proxy will handle all the external resources, caching and transforming data to our needs
    • New repo with the Proxy will be published asap
  • Code coverage and CI status is now handled by the proxy
  • Code blocks from github are now support in the embed shortcode using the Proxy
  • Code blocks with title or symbols have permalinks

{{<embed src="github:filecoin-project/specs-actors/actors/builtin/paych/paych_actor.go" lang="go" symbol="resolveAccount" title="Payment actor - Resolve Account">}}
Screenshot 2020-09-17 at 11 58 46

  • Code blocks from github have links to the github source page
    {{<embed src="github:filecoin-project/specs-actors/actors/builtin/paych/paych_actor.go" lang="go" symbol="resolveAccount">}}

Screenshot 2020-09-17 at 11 57 57

  • All Hugo Modules removed except the theme

No more codecov timeouts or go modules errors in the CI 🥳. The Proxy server should always keep data cached but fresh.

@hugomrdias hugomrdias linked an issue Sep 17, 2020 that may be closed by this pull request
@hugomrdias hugomrdias marked this pull request as ready for review September 17, 2020 12:34
README.md Outdated
- [External modules](#external-modules)
- [Solving Common problems](#solving-common-problems)
- [References](#references)
- [Filecoin Specification](#filecoin-specification)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some VS Code plugin insists on adding the h1 from this doc to the ToC... I had to turn it off locally. I don't think we want it in the ToC.

olizilla
olizilla previously approved these changes Sep 17, 2020
Copy link
Collaborator

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

This is great! It can go in as is. A couple of suggestions

  1. UI nit: Can we have the UI for code examples match the look of figures, by adding a subtle border and reducing the font size a little.

Screenshot 2020-09-17 at 14 48 57

vs

Screenshot 2020-09-17 at 14 48 46

2: UX nit... If the src attribute of the embed shortcode was just the github url, authors could copy and paste from their browser url bar when adding new embeds, and the markdown would more explicilty show what it was refering to.

{{<embed src="github:filecoin-project/specs-actors/actors/builtin/cron/cron_actor.go"  lang="go">}}
# vs
{{<embed src="https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/cron/cron_actor.go"  lang="go">}}

it seems nice to have the source code be a little less tied to the details of how the shortcode is currently implemented.

@hugomrdias
Copy link
Contributor Author

  1. 100% next PR will normalise those boxes and permalinks
  2. the first version was exactly like that, but i wanted to take out the git ref (the shortcode supports ref="hash") so we can release with pinned tags instead of just master. Maybe im overthinking what do you think ?

Thinking about it now, each release will have the master version at the time of the CI run maybe we don't need this.... To give you more context im thinking about building history/versioning based in the Github Releases and IPFS deploys.

@olizilla
Copy link
Collaborator

Oh nice, i see. Well yes, as you said, I think you could use the github URL and the api could give the permalink to the latest commit at build time that the code snippet was pulled from.

If an author wants to link to a specific commit that never changes, they can just use the URL for that commit on github.

@hugomrdias
Copy link
Contributor Author

the api could give the permalink to the latest commit at build time that the code snippet was pulled from.

Ah nice thats even better, didnt think about that. 💚 you are the best @olizilla

@hugomrdias
Copy link
Contributor Author

btw 34s npm run build in the CI 🥳

@hugomrdias hugomrdias merged commit 96062c9 into master Sep 24, 2020
@hugomrdias hugomrdias deleted the feat/api-proxy branch September 24, 2020 17:11
@yiannisbot
Copy link
Collaborator

This is fantastic @hugomrdias!!

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.

Getting data from remote sources into the spec
3 participants