Skip to content

Update linter version and install instructions in README.md #1081

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

Open
wants to merge 5 commits into
base: v3
Choose a base branch
from

Conversation

UnwashedMeme
Copy link
Member

Proposed changes

Without these changes I can't develop on v3 reliably; the make lint unit-test and the pre-push scripts fail. See the individual commits for more details of what/why they do.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • If applicable, I have updated any relevant documentation (README.md)
  • If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13
    • This is all non-agent changes (tests, docs)
    • I ran make test on Ubuntu 24.04 and doubt anyone has run make test on any of the above listed platforms recently because any distro that has an /etc/os-release that doesn't exactly match the test expectation will fail.

The only visible change is that the mdatagen issue is now linked. All
the rest of these are just markdownlint recommendations.
This should fail the local inner test, not the parent.
`make unit-test` was failing on tests for parsing hostinfo. E.g.

- VersionId: (string) (len=5) "22.04",
- Version: (string) (len=29) "22.04.5 LTS (Jammy Jellyfish)",
+ VersionId: (string) (len=5) "24.04",
+ Version: (string) (len=26) "24.04.2 LTS (Noble Numbat)",

My host system (where make is running) is Ubuntu 24.04.2 LTS (Noble
Numbat) so it picking that up.

After tracing this through I found that

- The test is mocking what releaseinfo should return: internal/datasource/host/info_test.go#L518
- mock is consumed at internal/datasource/host/info.go#L298
- but then, internal/datasource/host/info.go#L299-L306
  - readOsRelease info (which is different than the "host"?)
     - if this errors returns the mocked hostReleaseInfo, e.g. on mac
     - if this returns data (my ubuntu host) it is merged into the returned data

It looks like these tests generally haven't been run on a host that
has `/etc/os-release` and tests shouldn't be subject to host system
data changes like that.

I've just put in a bad path so that on ubuntu hosts it won't read
anything; this should match the existing behavior of running this on
macos hosts.
When the local go toolchain is go1.24 or newer make lint reports a
bunch of spurious errors. This newer version of golangci-lint is
compatible with go1.24 eliminating that.

- `context` config changed, there are now more options than true or
false. "" disables it to match the previous behavior
- `exported-is-used` is deprecated, it is always true now.
@UnwashedMeme UnwashedMeme requested a review from a team as a code owner May 15, 2025 19:07
@github-actions github-actions bot added bug Something isn't working chore Pull requests for routine tasks documentation Improvements or additions to documentation labels May 15, 2025
Copy link
Contributor

github-actions bot commented May 15, 2025

✅ All required contributors have signed the F5 CLA for this PR. Thank you!
Posted by the CLA Assistant Lite bot.

@UnwashedMeme
Copy link
Member Author

recheck

@UnwashedMeme
Copy link
Member Author

I have hereby read the F5 CLA and agree to its terms

@dhurley dhurley changed the title V3 fixups to be able to push Update linter version and install instructions in README.md May 21, 2025
@dhurley dhurley added the v3.x Issues and Pull Requests related to the major version v3 label May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chore Pull requests for routine tasks documentation Improvements or additions to documentation v3.x Issues and Pull Requests related to the major version v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants