CI fails, please fix.

Hi @wu-sheng,
Am I right that CI runs only after a manual approve of a maintainer? If yes, do you know why?

@halacs It is due to the fact that you're a newcomer to this repository, CI will be automatically run in your next contribution after this merges. :)

@Superskyyy I see :) I extended the tests a bit but I'm finished for sure now.

Hi @kezhenxu94, actually my change won't cause more file changes. My code just avoid duplicated headers. The way how we decide if a file needs to be updated remained the same. If a file header needs to be added, I check if there is an old header we need to remove first right before the new header would be added. Or maybe I don't understand something about the --replace CLI option.

Hi, @halacs , the config config.License.Pattern is used when there are existing license headers that are not perfectly the same with the content, but are acceptable so users don't need to modify those existing headers, with the changes in this PR, if users have configured config.License.Pattern, they will face more changes than expected, right?

Hi @kezhenxu94 ,

config.License.Pattern is used only in the rewriteContent function which is called only if expected license header is not there so it needs to be added or sometimes updated.

This means is expected license header is there file won't be updated so config.License.Pattern doesn't cause extra updates.

If I'm wrong please share where config.License.Pattern is also used.

Hi @kezhenxu94, actually my change won't cause more file changes. My code just avoid duplicated headers.

Can you show me a case of the duplicated headers, I think that should not happen.

If I understand correctly, this PR is to fix the problem above? Here is what I think: if the previous header satisfies the pattern, then no header will be prepended and thus no duplicate headers, if the existing header doesn't satisfy the pattern, how could you remove them via licensePattern.ReplaceAll(content, []byte(""))?

// Remove previous license header version to allow update it
 	if licensePattern != nil {
 		content = licensePattern.ReplaceAll(content, []byte(""))
 	}

hi @kezhenxu94,

This PR is to make license update easier. Think of the case when a few words, such as year range (e.g. 2016-2022) needs to be updated (e.g. to 2016-2023). We need to detect there is the license already but it is outdated so needs to be removed first.

In this case config.License.Content would be updated while the config.License.Pattern would make possible to remove the old one.

Until config.License.Content is intact header.satisfy(...) ensure that license header won't be changed at all so no file change, neither duplication.

However, if config.License.Content changed, original code added the license header even if it was already there (duplication). My PR implement is to avoid this case by using config.License.Pattern.

By the way, config.License.Pattern was already in the model but it was not used so far.

Hi @halacs thanks for elaborating, that totally makes sense to me now.

By the way, config.License.Pattern was already in the model but it was not used so far.

This must be bug when I was reorganizing the configurations, can you please also update the codes below

https://github.com/halacs/skywalking-eyes/blob/3c592e29166b21cf10ff6d92e809597b5afe2b3b/pkg/header/config.go#L107-L108

to

- pattern := config.Pattern
+ pattern := config.License.Pattern

and remove the unused config.Pattern field? If you want me to do I'm also OK to do later after this gets merged

Thanks!!!

I think the goal of this PR is clear now and I can merge it

Hi @kezhenxu94,

I just added the change you requested. I could run the make lint test build command locally to ensure all fine but I didn't have chance to retest it manually.

Thanks for your effort!