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

build: Configure deployment previews with Vega Editor #9294

Merged
merged 9 commits into from
Apr 6, 2024

Conversation

hydrosquall
Copy link
Member

@hydrosquall hydrosquall commented Mar 30, 2024

Motivation

Changes

  • Use explicit packageManager version so Cloudflare doesn't autoupgrade to yarn 3 (breaks lockfile)
  • Add a build script (originally this was a 1-liner in package.json but it was getting hard to read) that
    • Builds vega-lite
    • Clones vega/editor and links the local vega-lite in place of the full build
    • Install vega/editor without running the prepare script. (The CI device doesn't have rsync installed).
    • Makes empty index.json for vega / vega-lite examples.
    • Build editor with vite with / as the base URL (previously it was erroring due to thinking it was under the /editor subdomain). Technically this is an abstraction leak but fixing it would involve modifying the vega/editor repo too and I'd like to contain this change to 1 repo if possible.

Testing

  • See Cloudflare comment below, confirm site loads

Notes

Copy link

cloudflare-workers-and-pages bot commented Mar 30, 2024

Deploying vega-lite with  Cloudflare Pages  Cloudflare Pages

Latest commit: 566e4e8
Status: ✅  Deploy successful!
Preview URL: https://8c3d6b88.vega-lite.pages.dev
Branch Preview URL: https://cameron-yick-cloudflare-page.vega-lite.pages.dev

View logs

@hydrosquall hydrosquall self-assigned this Mar 30, 2024
@domoritz
Copy link
Member

What's the difference between preview and branch preview?

@hydrosquall
Copy link
Member Author

What's the difference between preview and branch preview?

  • Preview is per-commit, the URL changes each time I push a new commit
  • branch preview is per-branch, the URL is constant for the lifetime of this PR.

Since we know that this is now working, I'm going to remove the "console.log" commit so the linter won't be bothered.

@domoritz
Copy link
Member

domoritz commented Apr 2, 2024

I see. So the branch url will always give me the latest while the commit will always be the same snapshot. Looks good.

@hydrosquall hydrosquall changed the title (in progress) build: Configure deployment previews with the Vega Editor build: Configure deployment previews with Vega Editor Apr 4, 2024
@hydrosquall hydrosquall marked this pull request as ready for review April 4, 2024 22:29
@hydrosquall hydrosquall requested a review from a team as a code owner April 4, 2024 22:29
@hydrosquall
Copy link
Member Author

Apologies for the reping @domoritz - just had to move the PR out of draft.

I haven't heard back from the Cloudflare team RE: open source sponsorship, but I'm OK with just using my plan's builds for the month so we get a napkin math sense of how many builds per month we'll need for our usual contribution volume.

@hydrosquall hydrosquall requested a review from domoritz April 4, 2024 22:30
@mattijn
Copy link
Contributor

mattijn commented Apr 5, 2024

Will this trigger automatically on each PR and subsequent commits? I can imagine a PR on the docs don't require this preview. Would it be an idea to trigger it manually on request?

@domoritz
Copy link
Member

domoritz commented Apr 5, 2024

For now only cameron.yick/* branches have the previews since the build script is WIP. It's using my personal Cloudflare account for testing. I'll open it up to all (non dependabot/*) PRs this change is confirmed.

I think you switched this over, right @hydrosquall? Can we exclude docs: ... as well?

Either way, for now I think we should merge this to see how it works out.

Copy link
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

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

🎉

@hydrosquall
Copy link
Member Author

hydrosquall commented Apr 6, 2024

Will this trigger automatically on each PR and subsequent commits? I can imagine a PR on the docs don't require this preview.

Yes, good point. I can also ignore builds that only change files under a specific directory. I could set up a rule to ignore changes to the site/ and examples/ repos, but after inspecting it may be easiest to just build on changes to the src/ folder.

Would it be an idea to trigger it manually on request?

I don't see a straightforward way to do that yet. For now it'll just build on every commit.

I think you switched this over, right @hydrosquall? Can we exclude docs: ... as well?

Switching it over this evening! I'll have it exclude the docs/* path too

@hydrosquall hydrosquall merged commit 64d6daf into main Apr 6, 2024
10 of 11 checks passed
@hydrosquall hydrosquall deleted the cameron.yick/cloudflare-pages-test-spike branch April 6, 2024 02:03
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.

3 participants