Skip to content

remove unused buildkite pipeline#1098

Merged
richvdh merged 2 commits into
masterfrom
rav/del_bk_pipeline
Oct 29, 2021
Merged

remove unused buildkite pipeline#1098
richvdh merged 2 commits into
masterfrom
rav/del_bk_pipeline

Conversation

@richvdh
Copy link
Copy Markdown
Member

@richvdh richvdh commented Oct 21, 2021

the buildkite build is archived

Preview: https://pr1098--matrix-org-previews.netlify.app

@richvdh richvdh requested a review from afranke October 25, 2021 11:29
Copy link
Copy Markdown
Contributor

@afranke afranke left a comment

Choose a reason for hiding this comment

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

Overall this looks fine.
My main concern is that relying on docker images is not very idiomatic in the GHA world, and the same could be achieved using actions. Now that we figured how to cache things, we can use that for the gatsby node_modules and the custom image becomes moot.

I see advantages to the actions approach:

  • The images are basically unmaintained currently. Their sole purpose is this pipeline, so we’re the only ones using them and we hardly ever update them, with all the risks that it has.
  • Actions within the pipeline definition are more transparent than “stuff done outside”.
  • I have a hunch that there’s an overhead to the current image-based approach, but I don’t have scientific evidence to back it.

Does that make sense?

I don’t mean to put an additional burden on you and I volunteer to write the actions version if you think it’s worth pursuing, unless you want to tackle it yourself.

I also acknowledge the argument that we could merge this now and worry about improving upon it later, and we can do it if you think that’s the way to go. It feels a bit futile to move files just to remove them afterwards though.

@richvdh
Copy link
Copy Markdown
Member Author

richvdh commented Oct 27, 2021

I agree that having essentially unmaintained images is risky, and if you'd like to investigate better solutions, that sounds great to me.

Still, I'd rather merge this in the meantime than have unused things in the repo, which will be confusing for everyone.

@richvdh richvdh merged commit 2662123 into master Oct 29, 2021
@richvdh richvdh deleted the rav/del_bk_pipeline branch October 29, 2021 10:51
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.

2 participants