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

Move most utils and synthetic traits into core #1730

Merged
merged 3 commits into from
Sep 14, 2022
Merged

Move most utils and synthetic traits into core #1730

merged 3 commits into from
Sep 14, 2022

Conversation

jdisanti
Copy link
Collaborator

Motivation and Context

As part of refactoring more code into the core module, this PR:

  • Moves most of the util package from codegen-client into codegen-core
  • Moves the synthetic traits into codegen-core
  • Consolidates duplicated RustTypesTest files
  • Corrects lang -> rustlang package name in codegen-client tests
  • Moves letIf from SymbolVisitor into its own file in codegen-core

The Synthetics util was left in codegen-client since its unit test requires testutil, and testutil requires dozens more files from codegen-client. Pulling it in would easily quadruple the size of this PR.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jdisanti jdisanti requested review from a team as code owners September 13, 2022 00:35
@hlbarber
Copy link
Contributor

hlbarber commented Sep 13, 2022 •
edited
Loading

Should we move everything in the rustlang folder into core?

@jdisanti
Copy link
Collaborator Author

Should we move everything in the rustlang folder into core?

Yes, we should. Not in this PR though 😄 I'm trying to keep the changes smaller and easier to review. I attempted to move rustlang, but that ended up requiring a lot of smithy as well, so I decided to just stick with util for this PR.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti enabled auto-merge (squash) September 14, 2022 21:32
crisidev
crisidev approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@crisidev crisidev left a comment

Choose a reason for hiding this comment

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

Looks good!

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti merged commit e171ce0 into main Sep 14, 2022
@jdisanti jdisanti deleted the jdisanti-move-rustlang-utils branch September 14, 2022 22:03
david-perez added a commit that referenced this pull request Sep 22, 2022
@david-perez
Move more things into codegen-core
953ae53 
This moves most things from `codegen-client` that should be in
`codegen-core` into `codegen-core`.

This is a continuation of the efforts started in #1697, #1730.
@david-perez david-perez mentioned this pull request Sep 22, 2022
Merged
david-perez added a commit that referenced this pull request Sep 27, 2022
@david-perez
Move more things into codegen-core (#1762)
5b7aa7b 
This moves most things from `codegen-client` that should be in
`codegen-core` into `codegen-core`.

This is a continuation of the efforts started in #1697, #1730.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hlbarber hlbarber hlbarber approved these changes

@Velfi Velfi Velfi approved these changes

@crisidev crisidev crisidev approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@jdisanti @hlbarber @crisidev @Velfi