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

Go and CI maintenance - update deps, Go #27

Merged
merged 11 commits into from
Mar 20, 2024

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Mar 19, 2024

Remove appveyor and vagrant config.

Bump Go dependency versions.

Add testing with latest Go 1.22.x.

Fix staticcheck lint warnings as well as failure to execute under Go 1.22.

Remove GH action path-ignores' because "test" is a required check. When you change only .mdor.asciidoc` then you cannot merge the PR because the checks are missing.

@andrewkroh andrewkroh marked this pull request as draft March 19, 2024 19:48
File is very dated and the image it uses is no longer a supported OS by Go.
Don't run the staticcheck from the scripts because it needs to be
conditional upon the Go version being used. That is, in order to use newer
versions you need a newer Go.

Add a linter comment to the generated code to ignore usages of syscall.Syscall.
The Go team decided to no update existing code to use SyscallN so we will follow suit.
Certain GH checks are required in order to merge a PR.
If you modify a `.md` file then the checks are not triggered
and you cannot merge.
@andrewkroh andrewkroh force-pushed the project-maintenance branch from 8e23344 to 5f7efe0 Compare March 19, 2024 21:34
@andrewkroh andrewkroh marked this pull request as ready for review March 19, 2024 21:38
@andrewkroh andrewkroh requested a review from a team March 19, 2024 21:39
@andrewkroh
Copy link
Member Author

If anyone reviewing this PR has access to update the required CI checks for the repo, then please do so. You can see that it is still checking for test on Go 1.17.

@reakaleek
Copy link
Member

reakaleek commented Mar 20, 2024

If anyone reviewing this PR has access to update the required CI checks for the repo, then please do so. You can see that it is still checking for test on Go 1.17.

Hi @andrewkroh,

We can create a single status check, which can be set as required. This check would depend on the matrix's outcome.

Here is an example of such a single status check.

This way, we can also handle the docs (*.md) paths. Here is an example workflow that sets the test status check to success if only docs files are created.

I will approve this, update the required status checks for now, and create a follow-up applying the previously mentioned patterns.

@andrewkroh
Copy link
Member Author

I will approve this, update the required status checks for now, and create a follow-up applying the previously mentioned patterns.

@reakaleek That would awesome. Can you give me permissions to merge on this repo too (probably at the team level is best @elastic/sec-windows-platform)?

@reakaleek
Copy link
Member

I will approve this, update the required status checks for now, and create a follow-up applying the previously mentioned patterns.

@reakaleek That would awesome. Can you give me permissions to merge on this repo too (probably at the team level is best @elastic/sec-windows-platform)?

@andrewkroh you should now have appropriate permissions

@andrewkroh
Copy link
Member Author

@reakaleek It looks like there is some branch protection rule that is still preventing any merges.

The base branch does not allow updates.

Screenshot 2024-03-20 at 13 33 20

@amannocci
Copy link

@reakaleek It looks like there is some branch protection rule that is still preventing any merges.

The base branch does not allow updates.

Screenshot 2024-03-20 at 13 33 20

The main branch was locked without a particular reason.

@andrewkroh andrewkroh merged commit abac780 into elastic:main Mar 20, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants