Skip to content

Conversation

@oddgrd
Copy link
Contributor

@oddgrd oddgrd commented Jan 6, 2023

I started implementing this and the workflow works, but I haven't gotten to the website parts yet.

The workflow checks out the website and hyper, lowercases all the files in hyper/docs and cps them to _contrib. It then stages the changes, and if there are any staged changes it pushes a commit. Finally it pushes the commit (if there is one).

Workflow run: https://github.com/oddgrd/hyperium.github.io/actions/runs/3858409156/jobs/6576897139
Commit: oddgrd@3f258e3

Second run (no changes so no commit): https://github.com/oddgrd/hyperium.github.io/actions/runs/3858441335

For reference: https://github.com/actions/checkout#Push-a-commit-using-the-built-in-token

Closes hyperium/hyper#3100

@seanmonstar
Copy link
Member

Super cool! So, would this run on like a nightly basis? And, would it just automatically update the website? Or does it submit a PR we need to merge? Sorry if I missed a sentence explaining that...

@oddgrd
Copy link
Contributor Author

oddgrd commented Jan 6, 2023

Yes, it will run every night at 3am UTC (or if activated manually via cli/github UI), and it will automatically push a commit to master if there are any changes in _contrib.

I don't think it would be very hard to change it to send a PR instead, though. On one hand it makes me a little bit uncomfortable to have CI automatically push to master, on the other hand it is quite convenient. What would you prefer?

@seanmonstar
Copy link
Member

I agree with both of your feelings: spooky, but would approving duplicate PRs the following day just be annoying? If it submitted PRs, and we didn't pay attention, or overeagerly dismissed the email notification, would that mean the website and main repo fall out of sync? Which do you lean towards?

@oddgrd
Copy link
Contributor Author

oddgrd commented Jan 7, 2023

The current strategy should be safe since it only pushes changes to _contrib, but for a big project with many contributors it may cause problems down the line. I am leaning towards sending pull requests, and using an action to overwrite it if it already exists (if we forgot to merge the previous one).

I think the optimal solution would involve a webhook in the hyper repo to trigger the website workflow on push to hyper/docs, so we'd get the updated PR immediately rather than at a somewhat arbitrary later time. Working across repos adds a bit of complexity though.

I think we can start with the cron job PR version, if you're happy with that, and implementing the actual contrib section of the website. Then we can optimize it later with a web-hook. And when we've learned how to do this for the docs/contrib, we should be able to apply it easily to other sections that are duplicated between repos.

oddgrd and others added 3 commits January 7, 2023 22:56
also start implementing the contrib section, but it might need a new layout
is provided to hyper, and hyper acts on top of it. hyper returns `Future`s that
the user then decides how to poll, likely involving their runtime options.

![architecture diagram](./vision-arch.svg)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Links like this seem to work in production, but not locally. 🤷

This is the error locally: ERROR /contrib/vision/vision-arch.svg not found., and this is a working example that's live:

You already have a [Hello World server](hello-world.md)? Excellent! Usually,

@oddgrd
Copy link
Contributor Author

oddgrd commented Jan 9, 2023

Here is an example PR from the bot: oddgrd#1

oddgrd and others added 4 commits January 13, 2023 23:17
this is not ideal, but the script to lowercase links could be changed to allow MSRV name to be capitalized and links to be lowercased should we want to use MSRV as a link
@oddgrd oddgrd marked this pull request as ready for review January 13, 2023 22:32
Copy link
Contributor Author

@oddgrd oddgrd 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 essentially done @seanmonstar, except for a few minor details I left review comments for.

The workflow did end up a little bit complicated, which I know is not ideal for maintainability. I'm happy to volunteer to maintain it though, at least for a good while, and hopefully find some clever ways to make it simpler in the long run.

There is also the question of implementing a trigger in the main repo so the update PR is sent immediately, but I think this is an optimization that can be applied later.

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

I think this looks fantastic. I'll wait till tomorrow I'm case there's anything you wanted to do before I click merge. Otherwise, 🎉!

@seanmonstar seanmonstar merged commit 9688d22 into hyperium:master Jan 17, 2023
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.

Website: Add top-level Contrib section

2 participants