Hi Christopher,

I checked out the artifacts from the auto-built jobs.
The first thing I noticed was that downloading the artifacts took over five
minutes.
Now this may be a personal issue, but if this is the same for everyone then
downloading the files
could take longer than reviewing the actual changes.

Once I extracted the files, I was able to view a single rendered page's
contents without issue.
However, site navigation functionality or cross-page links would not work
as the links
are relative (file:///tour) instead of listing the full filepath
(file://~/site/tour)

Given the limited functionality, I personally don't think there is much
benefit to locally reviewing the current build artifacts.

Earlier you mentioned the risk of malicious content being committed.
I believe we can fully mitigate that risk with respect to outside sources
and keep the build process simple.
It could be done with the implementation of a modified gitflow workflow.

What do you think of this idea?

Github Actions:
The QA job action would be allowed to run on all public PRs against staging.
The staging publish action would only run as a post-merge action
<https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-your-pull_request-workflow-when-a-pull-request-merges>
on
the staging branch.
The asf-site publish action would only run as a post-merge action
<https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-your-pull_request-workflow-when-a-pull-request-merges>
on
the main branch.

Git merge flow:
feature_branch(s)->staging->main.

When making changes to the website, all PRs are pointed against the staging
branch.

Changes would be merged in to allow reviewing of the rendered content.
(Solves Dave's concerns)
ASF committers should be allowed to merge their PRs to staging as long as
the QA job succeeds (Merging to staging does not require excessive reviews
for minor changes).

External collaborators would need an existing committer to approve their
changes and merge them. This behavior would not change
Since the staging publish happens post-merge, all changes would be
reviewed, eliminating the possibility of malicious content being committed.

Once the collaborator is happy with the state of the staging branch, they
open a single PR from staging to main.
This allows fix-up actions to be completed without allowing the changes to
hit the "official site".

Benefits:
* Fast as the current process since committer changes to staging don't
require full reviews, only the staging to main PR.
* All publish actions are automated and the only additional complexity is a
second PR, keeping the process very simple.
* Allows for a full github action based-build process, removing the need
for ruby to be installed locally and reduces the amount of work needed to
make a change on the production site.
* Allows the process to go as fast or as slow as the committer wants
(external committers excluded).
* By using the staging branch to aggregate changes, small changes can be
batched up for a single release that
only costs a single CDN cache invalidation. (unknown if the current
accumulo site uses a CDN, so very minor benefit)



On Fri, Apr 7, 2023 at 10:02 PM Christopher <ctubb...@apache.org> wrote:

> Hi Dan,
>
> Auto-publishing a PR to a staging branch is probably not a good idea
> (I can see how it could be abused to put malicious content on the
> apache.org domain).
>
> Having some on-demand staging that is manually triggered could work,
> but I'm pretty sure that would lead to more complexity than what we
> have now, and right now it's very minimal.
>
> I'm not sure if this helps, but I've already implemented auto-building
> each PR, which you could view locally if you want. Check the
> https://github.com/apache/accumulo-website/actions for recently built
> site artifacts from PRs. Maybe that is a good option for some people
> that would make this proposed streamlining easier to accept?
>
>
> On Fri, Apr 7, 2023 at 7:39 PM Daniel Roberts <ddani...@gmail.com> wrote:
> >
> > +1 I definitely agree that merges to main should deploy to asf-site.
> > However, I think staging still has a place in the build pipeline, just
> not
> > where it currently lives.
> >
> > The staging branch provides a way to review rendered changes and does
> > remove the "worked on my machine" factor.
> > Reviews can cover both the markdown source and rendered code.
> >
> > So, could auto publishing to the staging branch be part of the QA build
> > step? Or maybe a manual build step being triggered?
> > I could see the latter being useful if multiple PRs were open at the same
> > time to avoid artifact publishing collisions.
> >
> > On Fri, Apr 7, 2023 at 6:34 PM Christopher Shannon <
> > christopher.l.shan...@gmail.com> wrote:
> >
> > > +1 to make the change. I agree that when PRs are merged we generally
> just
> > > want to publish immediately.
> > >
> > > FWIW, this is how we do things for the ActiveMQ website
> > > <https://github.com/apache/activemq-website> and it works fine, we
> just
> > > publish and there is no staging.
> > >
> > > On Fri, Apr 7, 2023 at 3:49 PM Christopher <ctubb...@apache.org>
> wrote:
> > >
> > > > Hi Accumulo Devs,
> > > >
> > > > When I first set up the automation using .asf.yaml for Jekyll builds
> > > > to go to the asf-staging branch, I expected us to get a bit more use
> > > > out of the staging site at https://accumulo.staged.apache.org/
> > > > However, that really hasn't happened, and it seems that we basically
> > > > almost always want to publish to the https://accumulo.apache.org
> once
> > > > we've approved a PR against the main branch and have a successful
> > > > Jekyll build.
> > > > Having the staging site is now just an unnecessary extra step to
> > > > publish (that we sometimes forget to do for a while, even though it's
> > > > trivial to run ./_scripts/publish.sh). I'm not sure it's adding any
> > > > value.
> > > >
> > > > Perhaps we should just have Jekyll build directly to the asf-site
> > > > branch and get rid of the asf-staging branch?
> > > >
> > > > The one benefit to the staging site is that, in theory, it's a
> > > > conscientious decision to publish. But, I don't think we've ever come
> > > > across any kind of situation where we wanted to stage the site, but
> > > > then didn't immediately want to publish it. So, I'm not convinced
> that
> > > > extra step is adding any value, especially since we're almost always
> > > > making the conscientious decision in GitHub when approving a PR to
> > > > update the site already.
> > > >
> > > > Any thoughts or opinions on this? I'm leaning slightly towards
> > > > streamlining this.
> > > >
> > > > Regards,
> > > > Christopher
> > > >
> > >
>

Reply via email to