fix: Use public modifier only when required by module type
#2313
Conversation
✅ Deploy Preview for apollo-ios-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| @@ -111,6 +111,14 @@ extension TemplateRenderer { | |||
| """ | |||
| ).description | |||
| } | |||
|
|
|||
| func embeddedAccessControlModifier( | |||
There was a problem hiding this comment.
This and the call sites into this function are the only relevant pieces of this change. The rest is passing the shared configuration reference around and formatting.
There was a problem hiding this comment.
At this point, are there even any TemplateRenderers left that don't need a reference to the config? I think we should just make config a property on the TemplateRenderer protocol. Then we could change this from a function you need to pass config into and just make it a computed property on the protocol.
All the functions above this become a lot cleaner too. This doesn't need to happen today to get this PR in. Since we wanna push the release ASAP. But I think i'm going to make that change sometime this week.
There was a problem hiding this comment.
Yes it would be a good change to get it into TemplateRenderer as you mentioned. It started out that not many templates needed to reference the configuration but it's way more complicated now.
|
Changing this to a draft PR because I need to rebase it on top of #2312 and catch up on the new/updated templates. |
calvincestari
changed the base branch from
release/1.0
to
1.0/fragment-conditions
calvincestari
force-pushed
the
1.0/2302-public-modifiers
branch
from
224daf3 to
a7878d4
Compare
| @@ -111,6 +111,14 @@ extension TemplateRenderer { | |||
| """ | |||
| ).description | |||
| } | |||
|
|
|||
| func embeddedAccessControlModifier( | |||
There was a problem hiding this comment.
At this point, are there even any TemplateRenderers left that don't need a reference to the config? I think we should just make config a property on the TemplateRenderer protocol. Then we could change this from a function you need to pass config into and just make it a computed property on the protocol.
All the functions above this become a lot cleaner too. This doesn't need to happen today to get this PR in. Since we wanna push the release ASAP. But I think i'm going to make that change sometime this week.
Tests/ApolloCodegenTests/CodeGeneration/Templates/EnumTemplateTests.swift
Outdated
Show resolved
Hide resolved
calvincestari
force-pushed
the
1.0/2302-public-modifiers
branch
from
a7878d4 to
58ed1c8
Compare
…red by module type
…ed by module type
…n required by module type
…n required by module type
…en required by module type
…ns when required by module type
calvincestari
force-pushed
the
1.0/2302-public-modifiers
branch
from
d048665 to
f71d41d
Compare
PR 1 of x for #2302 (Test mocks are still broken and require use of the namespace when
.embeddedInTargetis used)This PR now assesses the module type and only adds the
publicmodifier when a module is generated for the schema types. When the schema types are embedded within an existing target and automatically namespaced thepublicmodifier is redundant and generates compiler warnings on build.