Code Cleanup and Styling

I took a look at the code, and to the topics here.
I’ve seens a lot about refactoring / supporting new features. That is nice. But, nothing to cleanup the current code.

So, this is a starting point for the cleanup which, in my opinion, should be done before opensourcing the code.

  • RuntimeLibrairies / ExternalReferences : are references to specifics Windows Folder. The content of this folder are simply dll, where we are, today, unable to say clearly : “Which version of is used ?”. There should be a cleanup to move this part to nuget packages usage.
    GO Framework should be packable, to do that, in my mind, the projects should be set up to the new csproj format.
    New csproj style are much clearer, and reduce the amount of specific data. (Removing ItemGroup with the Compile of each file, for example)

  • Coding style / Code uniformization : While open sourcing, there will have common questions about the style to apply to the code. Should you use spaces or tabs ? 2 spaces length ? 4 spaces length ?
    How to ensure that everyone will use the same syntax ? Quite easy. .editorconfig file is here to do this.

  • Removing the dead code. Code inside Deprecated Code folder / Old / commented should not be in the current version. That the main purpose of the versionning system. If you need it at some point, you will be able to retrieve the code. Removing dead code will make potential evolution easier.

  • C# syntax uniformization. ie: String interpolation are not used everywhere.

  • Remove sensitive informations. In docker repository, I saw a password.

  • Adding README files. After a few years away from GO platform, it’s hard a bit difficult to know what is the purpose of each repository. Having a simple description might help contributors to know if this repository might interest them, or not.

Cleaning up the current Custom Code should be an entry point to apply this to the Generated Code.

1 Like

Thank you @Silas for the suggestion

Are you suggesting to potentially help on this topic ? If yes, that would be very welcome :slight_smile:

Yes indeed, we are using the old way to reference dependencies… to be upgraded. Maybe done after or during the migration to .NET Core ? @Garth working on that part (.NET core migration)

I don’t know what you call the new csproj, can you point us to some resource for this topic ? Probably also part of migration to .NET Core

Yes indeed good thing to have … and if we can auto-check/verify on post-build or something like this even better. This brings me to another related topic : we should make sure GO does not build with thousands of warnings. We should have no warning by default, so that when we have relevant warnings, we can actually notice them !

Yes also. Although not always easy to detect old dead code. We can start with the obvious one. And then we should have some code coverage tools that could be complementary to the unit test base, to both report on test coverage, but also on dead code

I already did a first cleanup, apparently missed some !

yes definitively part of what needs to be done / documentation. I took the opportunity to create another post around the selection of a documentation tool : What tools for documenting the Generative Objects project?

Yup. I changed “a bit” about code quality since we worked together. :smiley:

I have a set of very restrictives rules for Code Quality that I use on projects. I will try to apply this ruleset on go-framework first when I will have the ability to push a new branch.

Not really sure it is a good idea to make this in a single tasks. But, it’s just that…

I have a bad feeling about this.

Ruleset files / Roslyn Analyzers. :slight_smile:

Code quality rules should be set on Custom Code first, then applied on generated code.

1 Like

Ok great ! Thank you :pray:

Ok, can you expose here the set of rules you are proposing, so that we can all have an idea upfront ? Also : go-framework seems a good place to start, however be aware that by changing go-framework, you will have to change all the call in all the templates (go-codegenerator) that are using go-framework methods (if you change also the go-framework interface, as part of reviewing code quality), and potentially code in go-application too.

For your info also : we now have a regression test suite. So running the test will support you in verifying you did not break anything (although test coverage is still not that high)

And : can you confirm that removing all warnings that are currently generated from building the code (and also the framework) is part of the code quality pass ?

Today, my first pass is applying rules on go-framework, and, removing all errors / warnings (except uncalled methods, which might be used elsewhere, like OnBefore/OnAfter.)

What I have done so far is :
Replacing old school <Project toolsVersion="12.0" (...)> tag with the new <Project Sdk="Microsoft.NET.Sdk">
Allowing to remove all <Compile> tags and add <PackageReference>.
I have added few <PackageReferences> (completion in progress.)
I added a Restrictive Ruleset. Giving all the details of the ruleset hre might be very long. To make it short, it ensure that :

  • No multiple break lines.
  • Use string.empty instead of “”
  • Use type alias instead of String / Boolean / Int32 etc…
  • Removing unused imports, and sorting them.
  • Place brackets properly.
  • Always suround conditionnal expression and loops with brackets
  • Sort Getters then Setters.
  • Ensure case usage is correct, according to the visibility of a property.

While passing on every files, I created a specific file for each class. (even enums)

  • One class per file.
  • Switching explicit var declaration to var

On top of that, I am currently fixing Resharper errors about missing comments by adding a default TODO: to mask the issue for now, but to keep in mind that code documentation should be completed.

For now, I did not realy ran tests, because it’s mainly code styling, and not really a refactoring.
Impacts should be more or less quite null, because :

  • I don’t replace the dll in the repository. Replacing them seems to be a bad idea, and using standard packages might be a best idea. Since the tag modification allow us to pack directly the csproj.
  • Since it is mainly formatting, troubles should be very limited.

When I will be able to push, you will see that the core did not really changed, it is simply styling uniformization to comply with C# standards.

Yes, but I encountered several issues while trying to build, because I don’t have SD libraries. I will try to investigate that point soon.
Today, I only focus myself on removing warning and newly errors generated on the go-framework repository.

1 Like

Great, all feels good,

For the SD.* libraries, they are supposed to be in go-framework\ExternalReferences

There is a script “deployruntimelibraries.bat” in go-application\scripts to deploy all to the relevant place

I don’t know if the go-framework solution file is up to date. I always build the framework from the full generative objects solution that is in go-application