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

Fix warnings in codegen configuration documentation #125

Merged

Conversation

mt00chikin
Copy link
Contributor

@mt00chikin mt00chikin commented Nov 7, 2023

ApolloCodegenConfiguration's documentation had invalid references, which was causing the GenerateDocumentation command to fail.

[DEBUG - ApolloCodegenLib:main.swift:77] - Generating documentation for 'ApolloCodegenLib'...
Converting documentation...
/Users/t201595/src/apollo-ios-dev/apollo-ios-codegen/Sources/ApolloCodegenLib/ApolloCodegenConfiguration.swift:672:59: warning: 'EnumCase' doesn't exist at '/ApolloCodegenLib/ApolloCodegenConfiguration/ConversionStrategies'
/Users/t201595/src/apollo-ios-dev/apollo-ios-codegen/Sources/ApolloCodegenLib/ApolloCodegenConfiguration.swift:698:51: warning: 'CaseConversionStrategy' doesn't exist at '/ApolloCodegenLib/ApolloCodegenConfiguration'
/Users/t201595/src/apollo-ios-dev/apollo-ios-codegen/Sources/ApolloCodegenLib/ApolloCodegenConfiguration.swift:703:50: warning: 'CaseConversionStrategy' doesn't exist at '/ApolloCodegenLib/ApolloCodegenConfiguration'
Conversion complete! (0.29s)
Generated DocC archive at '/Users/t201595/src/apollo-ios-dev/docs/docc/ApolloCodegenLib.doccarchive'

The header docs were pointing to EnumCase and were missing an additional path component. This fixes those invalid references and re-generates the documentation.

Current state:
Screenshot 2023-11-07 at 13 59 08
Screenshot 2023-11-07 at 13 59 35

Updated:
Screenshot 2023-11-07 at 14 00 11
Screenshot 2023-11-07 at 14 00 07

@apollo-cla
Copy link

@mt00chikin: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@mt00chikin mt00chikin marked this pull request as draft November 7, 2023 18:39
@mt00chikin mt00chikin marked this pull request as ready for review November 7, 2023 19:24
@mt00chikin mt00chikin changed the title Fix broken references in codegen configuration documentation Fix warnings in codegen configuration documentation Nov 7, 2023
Copy link
Member

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

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

Thanks for fixing these warnings @mt00chikin. There are a lot of documentation changes in here that haven't gone out in a release yet though. The reason for that is the documentation is built as soon as the PR is merged whereas releases are from a tag.

So to fix this:

  1. Create a new branch from the 1.7.0 tag,
  2. Redo your edits to ApolloCodegenConfiguration,
  3. Re-generate the documentation

That should include only the fixes to the codegen configuration and not the other stuff we haven't released yet - thanks.

@calvincestari
Copy link
Member

You'll have to end up force pushing if you keep this branch name or just close this PR and create an entirely new one.

@mt00chikin
Copy link
Contributor Author

No prob. Thanks for the info! I was wondering why the documentation was so different on my branch, that explains it. I'll try to get this updated soon.

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.

Thanks so much for this! And good catch @calvincestari. We'll get this in as soon as you push the update!

@mt00chikin mt00chikin force-pushed the fix-codegen-documentation branch from 99ff51f to 1996958 Compare November 8, 2023 19:28
Copy link
Member

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

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

Thanks for pushing the update @mt00chikin, it's a more concise diff now - thanks.

@calvincestari calvincestari merged commit 0c0c4f6 into apollographql:main Nov 8, 2023
11 of 12 checks passed
BobaFetters pushed a commit to apollographql/apollo-ios-codegen that referenced this pull request Nov 8, 2023
BobaFetters pushed a commit that referenced this pull request Nov 8, 2023
10a0738e Fix warnings in codegen configuration documentation (#125)

git-subtree-dir: apollo-ios-codegen
git-subtree-split: 10a0738e7139125ddeff9ebd91fc6887da4fbfb0
BobaFetters pushed a commit that referenced this pull request Nov 8, 2023
…ration documentation

git-subtree-dir: apollo-ios-codegen
git-subtree-mainline: 84c0392
git-subtree-split: 10a0738e7139125ddeff9ebd91fc6887da4fbfb0
@mt00chikin
Copy link
Contributor Author

No prob! I saw that a test failed, but I noticed that my change has an invalid reference 🤦 I'm gonna fix it and get one more commit up

@mt00chikin
Copy link
Contributor Author

Or not ;)

@calvincestari
Copy link
Member

The failed test was in the pagination module though and didn't look related to the docs change. So I merged it anyways.

@mt00chikin
Copy link
Contributor Author

Sounds good. I accidentally left the old CaseConversionStrategy in the path for the EnumCases header doc.
Was gonna remove that, but I could just make that change in a pull request against main and the documentation should eventually pick it up.

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.

5 participants