Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GOOS=windows to lint-matrix #30743

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

pjanotti
Copy link
Contributor

Description:

Add a lint step to CI with GOOS=windows. This will ensure that code for Windows keeps a clean lint pass.

Until the lint Go cache is invalidated the step with GOOS=windows will be a bit longer than the Linux step. I'm measuring in my fork to see what we should expect after the cache is updated. I had an alternative implementation using Windows runners instead, but, overall it required separated Go caches and many more runners, running on Linux also ensures that the step can also be run by contributors on their dev boxes.

Link to tracking Issue:
Related to #11557 and #27866, but, it doesn't fix any of the two.

Testing:
Validated that the step is passing for all components

Documentation:
N/A

@pjanotti pjanotti requested review from a team and Aneurysm9 January 23, 2024 21:53
@pjanotti pjanotti changed the title Add GOOS=windows to lint-matrix step Add GOOS=windows to lint-matrix Jan 23, 2024
@pjanotti
Copy link
Contributor Author

Confirmed: when the Go cache for lint is updated the step for GOOS=windows takes a bit less than the lint step with it. So until the cache is updated we should expect the step with GOOS=windows to take quite more, that, will improve after the PR is merged and the old caches get evicted.

@atoulme atoulme added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 24, 2024
@dmitryax dmitryax merged commit 6a2e178 into open-telemetry:main Jan 24, 2024
89 of 90 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 24, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Feb 1, 2024
**Description:**

Add a lint step to CI with `GOOS=windows`. This will ensure that code
for Windows keeps a clean lint pass.

Until the lint Go cache is invalidated the step with `GOOS=windows` will
be a bit longer than the Linux step. I'm measuring in my fork to see
what we should expect after the cache is updated. I had an alternative
implementation using Windows runners instead, but, overall it required
separated Go caches and many more runners, running on Linux also ensures
that the step can also be run by contributors on their dev boxes.

**Link to tracking Issue:**
Related to open-telemetry#11557 and open-telemetry#27866, but, it doesn't fix any of the two.

**Testing:**
Validated that the step is passing for all components
codeboten pushed a commit to open-telemetry/opentelemetry-collector that referenced this pull request Aug 1, 2024
I'm volunteering myself to own the tier 2 support for windows/amd64.
This will put the windows/amd64 on par with the other tier 2 platforms
by having a specific person owning it.

Some of Windows related things that I contribute(d):

* Codeowner of various Windows related components in contrib
(pkg/winperfcounters, activedirectorydsreceiver, iisreceiver,
windowseventlogreceiver, and windowsperfcountersreceiver)
* Fixing Windows related issues (e.g.: #9042, #9689, #9726,
open-telemetry/opentelemetry-collector-contrib#30743)
* MSI on the collector releases repo
(open-telemetry/opentelemetry-collector-releases#560)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants