krzysztofzablocki / Sourcery

Meta-programming for Swift, stop writing boilerplate code.
http://merowing.info
MIT License
7.58k stars 605 forks source link

Remove Sourcery version from header #1309

Closed dcacenabes closed 3 months ago

dcacenabes commented 3 months ago

As discussed in issue #399 , including the Sourcery version in the generated files headers has side effects, like higher CI times due to unnecessary cache invalidation.

This PR removes any references to the version as part of the file headers

art-divin commented 3 months ago

@dcacenabes Hello 👋🏻

Thank you very and very much for being initiative and creating this PR so quick!

I wonder, can we have such a strategy to have a runtime flag, which would do what you did here? I do not want to trigger such a huge diff for all consumers of Sourcery 2.1.9 due to this change, rather, we can provide this feature optionally via a flag, also adding a test covering this option.

What do you think?

dcacenabes commented 3 months ago

@dcacenabes Hello 👋🏻

Thank you very and very much for being initiative and creating this PR so quick!

I wonder, can we have such a strategy to have a runtime flag, which would do what you did here? I do not want to trigger such a huge diff for all consumers of Sourcery 2.1.9 due to this change, rather, we can provide this feature optionally via a flag, also adding a test covering this option.

What do you think?

Makes total sense! It will take me a little bit longer, since I am not familiar with the codebase and I need to understand it a little bit better.

dcacenabes commented 3 months ago

Just to make sure we're on the same page, you are suggesting to pass a command line option, something like force-parse or cacheBasePath ?

Edit: More along the lines of logBenchmarks and parseDocumentation https://github.com/krzysztofzablocki/Sourcery/blob/7a12709fcf07b4039bdf9e186d17aa9a9ca56274/SourceryExecutable/main.swift#L97-L98 , which are flags

art-divin commented 3 months ago

Just to make sure we're on the same page, you are suggesting to pass a command line option, something like force-parse or cacheBasePath ?

Yes, that would be perfect. To make things easier for you, ask all questions you have, sooner you'll ask, sooner I'll be able to accommodate you with an answer 🤝

dcacenabes commented 3 months ago

Just to make sure we're on the same page, you are suggesting to pass a command line option, something like force-parse or cacheBasePath ?

Yes, that would be perfect. To make things easier for you, ask all questions you have, sooner you'll ask, sooner I'll be able to accommodate you with an answer 🤝

I pushed some changes. I also added a test although not sure it is the best context to add it. Happy to apply any feedback you have to the implementation.

art-divin commented 3 months ago

I also added a test although not sure it is the best context to add it.

Oh well, one can never be sure, there are 600 behavioural tests and one can only guess where to put the test. But once you write it, if it becomes too much of a hassle to put it into that specific Spec file, or everything around looks too "unrelated" - then you might want to consider other Spec files for its placement - my approach is like this to this day.

art-divin commented 3 months ago

I will merge this, and before the new release, will test with the automation if some regex needs to be adjusted in the scripting.

Thank you very much 🤝 @dcacenabes and welcome to the Contributor club 🥳 🔥

dcacenabes commented 3 months ago

I will merge this, and before the new release, will test with the automation if some regex needs to be adjusted in the scripting.

Thank you very much 🤝 @dcacenabes and welcome to the Contributor club 🥳 🔥

Very happy to be a contributor! 🎉 Thanks to you for your support and fast response!

visma-ios-if commented 2 months ago

@dcacenabes I know this Pr is closed for some time now. But I am not able to figure it out how can I actually make use of this new flag hideVersionHeader in my sorcery .yml file, note, I am using the lates Sorcery release 2.2.3 ( I am seeing in the header) I tried to use it like this:

sources:
  - {path to my sources}
templates:
  - {path to stencil file template}
output:
  {output path}
hideVersionHeader: true

But it do not matter how many times I am trying to generate the file, the header comments including the version are in there. Can you give an example on how to use it?

art-divin commented 2 months ago

Hello @visma-ios-if ,

thank you very much for raising this!

I've found out that this was not supported, and so I have added this feature in #1328. Please see PR's description for the format.

dcacenabes commented 2 months ago

Thanks for taking care of this!