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

refactor: Remove unnecessary duplicate types from Types.ts #622

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

georgefst
Copy link
Contributor

If I'd reviewed #547 more thoroughly, I might've noticed that we no longer needed these duplicate types, which previously existed to work around our awkward dependency graph.

But then, that PR did touch 151 files.

@georgefst georgefst added the Ready to merge Ready to merge label Nov 14, 2022
mergify bot added a commit that referenced this pull request Nov 14, 2022
@mergify
Copy link

mergify bot commented Nov 14, 2022

Hey @georgefst, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with @mergifyio refresh.
More details can be found on the Queue: Embarked in merge train check-run.

@brprice
Copy link
Contributor

brprice commented Nov 14, 2022

I suspect this conflicts with #611, given that it is complaining about

Module '"./Types"' has no exported member 'GlobalName'.

@georgefst
Copy link
Contributor Author

I suspect this conflicts with #611, given that it is complaining about

Module '"./Types"' has no exported member 'GlobalName'.

Yep, I'll rebase.

@brprice
Copy link
Contributor

brprice commented Nov 14, 2022

I suspect this conflicts with #611, given that it is complaining about

Module '"./Types"' has no exported member 'GlobalName'.

Yep, I'll rebase.

I think I messed up that pr and used the GlobalName from Type.ts, rather than globalName.ts, so you may also need to adjust that -- we should probably still delete this GlobalName iiuc.

Sorry for the extra work!

@georgefst
Copy link
Contributor Author

No worries, I can't blame you - the state of things before this change was quite confusing.

@georgefst georgefst force-pushed the georgefst/no-duplicate-types branch from 9420090 to b01bcfd Compare November 14, 2022 13:10
In fact, some of these types no longer even exist in the backend / primer-api.
@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: ae1e77f
Status: ✅  Deploy successful!
Preview URL: https://677457fe.primer-app.pages.dev
Branch Preview URL: https://georgefst-no-duplicate-types.primer-app.pages.dev

View logs

mergify bot added a commit that referenced this pull request Nov 14, 2022
@mergify mergify bot merged commit 4752465 into main Nov 14, 2022
@mergify mergify bot deleted the georgefst/no-duplicate-types branch November 14, 2022 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge Ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants