Conversation
mohanmanikanta2299
changed the title
sonamtenzin2
left a comment
sonamtenzin2
left a comment
There was a problem hiding this comment.
These changes should be kept either on a separate branch or in a separate fork, as they are specific to hashicorp/ibm.
A feature that replaces one copyright header with another would be more generic and therefore more suitable for addition into main branch instead of this
| } | ||
|
|
||
| // Check for copyright regex patterns primarily in the top section | ||
| copyrightRegex := regexp.MustCompile(`copyright\s+\d{4}`) |
There was a problem hiding this comment.
Can the rest of the functionality be reduced to something like checking for the copyright statement in top text and then further checking for only if strings.Contains(topContent, "hashicorp") || strings.Contains(topContent, "ibm corp") ?
|
|
||
| // processFileUpdate processes a file for the update command, which handles both | ||
| // adding headers to files without them and replacing HashiCorp headers with IBM headers | ||
| func processFileUpdate(f *file, t *template.Template, license LicenseData, checkonly bool, verbose bool, logger *log.Logger) error { |
There was a problem hiding this comment.
Isn't hclog being used for logging in this repository ?
| // This is where copyright headers typically appear | ||
| topContent := content | ||
| if len(content) > 300 { | ||
| topContent = content[:300] |
There was a problem hiding this comment.
We should check for the first 1000 chars because I think in the rest of the places it checks for about the same number
| } | ||
|
|
||
| // Global variables to store the target license data for comparison | ||
| var targetLicenseData *LicenseData |
| } | ||
|
|
||
| // Check if both years were explicitly provided (even if same) | ||
| if strings.HasPrefix(targetYear, "EXPLICIT_BOTH:") { |
There was a problem hiding this comment.
Can you add a godoc comment detailing on the different tags that can be used in targetyear ?
| var wg errgroup.Group | ||
| for f := range ch { | ||
| f := f // https://golang.org/doc/faq#closures_and_goroutines | ||
| wg.Go(func() error { |
There was a problem hiding this comment.
Suggestion: Instead of creating a goroutine everytime, may be we can have some existing pool of goroutines which will pick up files from the channel. This will keep the numbers of goroutines running at a time bounded
2 participants
🛠️ Description
🔗 External Links
https://hashicorp.atlassian.net/browse/IND-5575
👍 Definition of Done
🤔 Can be merged upon approval?
✅
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.