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.

Sign up for GitHub

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

Jump to bottom

[nfc] replace glob with explicit file list #2012

Merged
merged 1 commit into from
Apr 12, 2024
Merged

[nfc] replace glob with explicit file list #2012

merged 1 commit into from
Apr 12, 2024

Conversation

mikea
Copy link
Collaborator

@mikea mikea commented Apr 12, 2024

Also remove url.c++ from "jsg" library since it is part of "url". Glob before didn't make it obvious that url.c++ is included twice.

@mikea mikea requested a review from fhanau April 12, 2024 20:04
@mikea mikea requested review from a team as code owners April 12, 2024 20:04
@fhanau
Copy link
Collaborator

fhanau commented Apr 12, 2024

Looks like src/workerd/jsg/jsg.h expects to have url.h in the same target, causing tests to fail 🙄. Changing the include to workerd/jsg/url.h should be sufficient to fix that.
Change LGTM, I was actually working on something similar. There's more things going wrong here (e.g. jsg-test.h being part of the jsg target), but that can be addressed in another cleanup PR.

@mikea
[nfc] replace glob with explicit file list
02fdbe0 
Also remove url.c++ from "jsg" library since it is part of "url".
Glob before didn't make it obvious that url.c++ is included twice.
@mikea mikea force-pushed the maizatskyi/2024-04-12-exclude-url branch from 341965d to 02fdbe0 Compare April 12, 2024 20:30
@mikea mikea merged commit b52ddb0 into main Apr 12, 2024
10 checks passed
@mikea mikea deleted the maizatskyi/2024-04-12-exclude-url branch April 12, 2024 21:08
This was referenced Apr 13, 2024
Merged
Merged
garrettgu10 pushed a commit that referenced this pull request Apr 17, 2024
@mikea @garrettgu10
[nfc] replace glob with explicit file list (#2012)
cc31c23 
Also remove url.c++ from "jsg" library since it is part of "url".
Glob before didn't make it obvious that url.c++ is included twice.
garrettgu10 pushed a commit that referenced this pull request May 13, 2024
@mikea @garrettgu10
[nfc] replace glob with explicit file list (#2012)
b907fd9 
Also remove url.c++ from "jsg" library since it is part of "url".
Glob before didn't make it obvious that url.c++ is included twice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhanau fhanau fhanau approved these changes

@Frederik-Baetens Frederik-Baetens Awaiting requested review from Frederik-Baetens Frederik-Baetens is a code owner automatically assigned from cloudflare/workers-runtime-1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mikea @fhanau