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 SchemaTypesFileOutput.ModuleType #2194

Merged
merged 3 commits into from
Mar 10, 2022

Conversation

calvincestari
Copy link
Member

@calvincestari calvincestari commented Mar 9, 2022

PR 1 of 3 for #2034

  • Refactors ModuleType to be more general in the third-party dependency managers that can be supported.
  • Refactored ApolloCodegenConfigurationTests with more reusable code, dropped a few tests that are no longer relevant with the module helpers being removed.

Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

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

Assuming we resolve the error thrown from SchemaModuleFileGenerator in the next PR, this looks great!

/// Use this option for dependency managers, such as CocoaPods or Carthage. Example usage
/// would be to create the podspec file (CocoaPods) or Xcode project file (Carthage) that
/// is expecting the generated files in the configured output location.
case other
Copy link
Contributor

Choose a reason for hiding this comment

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

Great job on these docs!!

@@ -10,10 +10,10 @@ struct SchemaModuleFileGenerator {
_ config: ApolloCodegenConfiguration.SchemaTypesFileOutput,
fileManager: FileManager = FileManager.default
) throws {
switch config.dependencyAutomation {
case let .swiftPackageManager(moduleName):
switch config.moduleType {
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now, if you chose other or none for moduleType, it's going to throw an error. If we are only supporting SPM, we need this to no-op on those moduleType values, not throw an error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, good catch. I'll relax the error for .other in #2195 and then relax the last one in the final 3rd PR for .none (still to come).

@calvincestari calvincestari merged commit ed928a2 into release/1.0 Mar 10, 2022
@calvincestari calvincestari deleted the 1.0/schema-types-module-types branch March 10, 2022 21:48
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