Make ODataT4CodeGenerator public

[KenitoInc]

Currently, if we run custom tool on the tt template, we get a diff that changes the ODataT4CodeGenerator modifier from public to internal. This is because the .cs file was modified but the tt file wasn't. This PR fixes that issue.

[gathogojr]

@KenitoInc @ElizabethOkerio Was there any reason we didn't go the friend assemblies route to make the internal methods accessible across the assemblies? If that is feasible, I'd honestly prefer that rather than making the class public

[ElizabethOkerio]

@KenitoInc @ElizabethOkerio Was there any reason we didn't go the friend assemblies route to make the internal methods accessible across the assemblies? If that is feasible, I'd honestly prefer that rather than making the class public

@gathogojr I remember some of the discussions that we had was to make the codegen a stand-alone library with the core functionalities for generating code. This meant that anybody with this library can or could provide their own interface on how they want to access it. That was the reasoning for making some of the methods or classes public.

[gathogojr]

@KenitoInc @ElizabethOkerio Was there any reason we didn't go the friend assemblies route to make the internal methods accessible across the assemblies? If that is feasible, I'd honestly prefer that rather than making the class public

@gathogojr I remember some of the discussions that we had was to make the codegen a stand-alone library with the core functionalities for generating code. This meant that anybody with this library can or could provide their own interface on how they want to access it. That was the reasoning for making some of the methods or classes public.

@ElizabethOkerio I'm not talking about every other class. I'm talking about the ODataT4CodeGenerator that is generated by running the TT file.
Actually for the same reason that you gave - anybody with this library can or could provide their own interface on how they want to access it - by making that this particular class public, aren't we setting ourselves up since it's prone to constant mutation as we accommodate additional functionality? The fact that they'd be affected by such changes is what makes me think it's not a good idea in this particular case

[ElizabethOkerio]

@KenitoInc @ElizabethOkerio Was there any reason we didn't go the friend assemblies route to make the internal methods accessible across the assemblies? If that is feasible, I'd honestly prefer that rather than making the class public

@gathogojr I remember some of the discussions that we had was to make the codegen a stand-alone library with the core functionalities for generating code. This meant that anybody with this library can or could provide their own interface on how they want to access it. That was the reasoning for making some of the methods or classes public.

@ElizabethOkerio I'm not talking about every other class. I'm talking about the ODataT4CodeGenerator that is generated by running the TT file. Actually for the same reason that you gave - anybody with this library can or could provide their own interface on how they want to access it - by making that this particular class public, aren't we setting ourselves up since it's prone to constant mutation as we accommodate additional functionality? The fact that they'd be affected by such changes is what makes me think it's not a good idea in this particular case

But they could always update to the new version if they want to. If they don't need the changes then they can continue using the older versions. Also we want this class to be accessible from outside the assembly and the whole idea of making things internal is to make them inaccessible from an assembly.

[habbes]

@gathogojr I remember some of the discussions that we had was to make the codegen a stand-alone library with the core functionalities for generating code. This meant that anybody with this library can or could provide their own interface on how they want to access it. That was the reasoning for making some of the methods or classes public.

While codegen is a stand-alone library, are we actually shipping it as a standalone library on NuGet? I assume not? Under which circumstances does the user interact with the class? I'm not sure I understand why we need to make it public. Do we want to make it public so it can be accessible by 3rd party code, or so that it can be accessible to the other projects, like the CLI or Connected service? cc @KenitoInc

[KenitoInc]

I agree with @habbes , since we aren't shipping codegen as a standalone library on nuget, we should leave the ODataT4CodeGenerator class as internal. When a time will come to ship the codegen as a standalone library, we will make ODataT4CodeGenerator class public and determine which appropriate methods to be public, internal and public virtual as well.

[ElizabethOkerio]

I agree with @habbes , since we aren't shipping codegen as a standalone library on nuget, we should leave the ODataT4CodeGenerator class as internal. When a time will come to ship the codegen as a standalone library, we will make ODataT4CodeGenerator class public and determine which appropriate methods to be public, internal and public virtual as well.

I thought @habbes was asking why we are making the class public ? We are making the class public because we want it to be accessed by the CLI and connected service. My question now is? what are we really protecting in this class. We want it to be accessed from outside codegen but we are insisting on making it internal?

[gathogojr]

I agree with @habbes , since we aren't shipping codegen as a standalone library on nuget, we should leave the ODataT4CodeGenerator class as internal. When a time will come to ship the codegen as a standalone library, we will make ODataT4CodeGenerator class public and determine which appropriate methods to be public, internal and public virtual as well.

I thought @habbes was asking why we are making the class public ? We are making the class public because we want it to be accessed by the CLI and connected service. My question now is? what are we really protecting in this class. We want it to be accessed from outside codegen but we are insisting on making it internal?

@ElizabethOkerio That's exactly what we're talking about. If the only other libraries we want to access this class are our own (CLI and Connected Service), and not 3P libraries, it makes sense to make CLI and Connected Service libraries be "friends" with the CodeGen library using InternalsVisibleTo. Making it public is not the only option

[ElizabethOkerio]

I agree with @habbes , since we aren't shipping codegen as a standalone library on nuget, we should leave the ODataT4CodeGenerator class as internal. When a time will come to ship the codegen as a standalone library, we will make ODataT4CodeGenerator class public and determine which appropriate methods to be public, internal and public virtual as well.

I thought @habbes was asking why we are making the class public ? We are making the class public because we want it to be accessed by the CLI and connected service. My question now is? what are we really protecting in this class. We want it to be accessed from outside codegen but we are insisting on making it internal?

@ElizabethOkerio That's exactly what we're talking about. If the only other libraries we want to access this class are our own (CLI and Connected Service), and not 3P libraries, it makes sense to make CLI and Connected Service libraries be "friends" with the CodeGen library using InternalsVisibleTo. Making it public is not the only option

@gathogojr Am not against making this class internal. My argument stems from the meaning of making a class internal.. We make a class internal when we want it to be accessed only within the assembly. In this case we are sure we want the class to be accessed from outside the assembly but we still want to make it internal. 🤔

[g2mula]

Has the class already shipped? If so, then the ship (on internal vs public) may already have sailed (pun intended)
suggestion: Let's use this opportunity to fix the spacing/indentation issues in the generated PR to avoid further diffs due to formatting code...