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

When a packed version of TypeScript is requested, request a build of Monaco #33536

Merged
merged 1 commit into from
Sep 20, 2019

Conversation

orta
Copy link
Contributor

@orta orta commented Sep 20, 2019

This adds a hook to trigger the generation of a build of Monaco-typescript and Monaco-editor from the pack comment inside the TS repo.

Here's an example build which I triggered pretending it was coming from #33290

All the deploy work is explained in https://github.com/orta/make-monaco-builds/

…t to create a build of monaco for the playground
@weswigham
Copy link
Member

Why not just amend the end of the tarball build pipeline to also fire off a Monaco build?

@weswigham
Copy link
Member

weswigham commented Sep 20, 2019

I mean, I guess this effectively does that. Eh, may as well include it in this script, rather than the job itself, I suppose.

But! You should include the tarball url in a query param/environment var, rather than trying to scrape the comment after getting the request, imo.

@orta
Copy link
Contributor Author

orta commented Sep 20, 2019

I wish I could send the actual URL, there's a limit to how many characters I can send (100 max) and the UUIDs basically eat all of that. Even after dropping all the args + urls which I could infer on the other size I couldn't get it safely under that number.

I'm happy moving it anywhere else though, curl will work just as well but it does rely on reading those comments so it kinda needs to be sure it was posted. Perhaps I can add it as an extra step in the azure devops job?

@weswigham
Copy link
Member

I wish I could send the actual URL, there's a limit to how many characters I can send (100 max) and the UUIDs basically eat all of that. Even after dropping all the args + urls which I could infer on the other size I couldn't get it safely under that number.

Yick. Can you include more characters as a POST body, rather than a GET query param?

@weswigham
Copy link
Member

I'm happy moving it anywhere else though, curl will work just as well but it does rely on reading those comments so it kinda needs to be sure it was posted. Perhaps I can add it as an extra step in the azure devops job?

Naaaah, this is fine. It's only used as the last stage of that job, anyway.

@orta
Copy link
Contributor Author

orta commented Sep 20, 2019

Nope, the request fails if you include anything other than that 100 char string, tried a few ways to get info between them but in the end this the most stateless/decoupled way I could think of

@orta orta merged commit ee6f00d into microsoft:master Sep 20, 2019
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