-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
feat: buildless ecosystem ci #18525
Open
Aslemammad
wants to merge
57
commits into
vitejs:main
Choose a base branch
from
Aslemammad:feat/buildless-ecosystem-ci
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat: buildless ecosystem ci #18525
Changes from all commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
93196f0
init
Aslemammad 582d059
debug
Aslemammad 406b2da
debug
Aslemammad 537d9f1
debug
Aslemammad fa83bca
debug
Aslemammad 7c88c4c
debug
Aslemammad 88cd5f5
debug
Aslemammad 40f55e1
debug
Aslemammad abec9fb
trigger ci
sapphi-red f067ea6
debug
Aslemammad de78f9f
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 560222a
debug
Aslemammad 596b846
debug
Aslemammad de824cb
debug
Aslemammad ac51d2b
debug
Aslemammad b7c60df
debug
Aslemammad 50dd0f4
trigger ci
sapphi-red 399ce9d
debug
Aslemammad cdce56b
debug
Aslemammad 444fc3a
debug
Aslemammad b48d3f4
debug
Aslemammad 51e2efc
debug
Aslemammad a1fd622
debug
Aslemammad d560a03
debug
Aslemammad 486135e
debug
Aslemammad a1ddb07
debug
Aslemammad e1fcdf9
debug
Aslemammad d841324
debug
Aslemammad ba291ed
debug
Aslemammad 4ecbede
debug
Aslemammad b60a576
separation
Aslemammad 66ee1a4
separation
Aslemammad 396d523
separation
Aslemammad b1cfd85
back to basics
Aslemammad 62faf78
finalize
Aslemammad c97f0eb
better way to remove reactions
Aslemammad 5406307
revert preview-release.yml
Aslemammad ec00b0a
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 4edf2c4
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 00c68c6
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 5f96097
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 5395d82
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 8127bfc
Update .github/workflows/ecosystem-ci-trigger.yml
Aslemammad 6dfc0f9
move permissions check to the top
Aslemammad 9c747f1
debug git show
Aslemammad 1e46ed6
debug git show
Aslemammad f52e5e9
debug git show
Aslemammad d7bedb8
debug git show
Aslemammad 1ff8a14
debug git show
Aslemammad 2101400
debug git show
Aslemammad a32b13f
debug git show
Aslemammad 5c07c4c
debug git show
Aslemammad c65eb11
debug git show
Aslemammad 73708df
debug git show
Aslemammad eb3e33b
debug git show
Aslemammad 0183d18
debug git show
Aslemammad 7615586
find collisions properly
Aslemammad File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not yet implement the
HEAD
for pkg.pr.new! So here instead of consuming the request body and waiting for it, we just check the status and then the step would be done.So if it was fetching, it'd be aborted I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @dominikg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you await fetch(), the full file has been downloaded already, its just not been parsed yet. of course if pkg.pr.new does not implement head we have to go with full, but should be considered as an optimization. or do you implement the endpoints about package information that the public registry has?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not know that, does not that defeat the purpose of streams (e.g.
.body
)?As far as I know the await on
.body
would wait until finishing the fetch and meanwhile give us the result buffer by buffer while fetching.💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, maybe need to get more coffee and you are right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, thanks, let me get my own coffee too now 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any plan to support it? It sounds interesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create an issue for it then!