Fix Signal Issue on Windows #1159

H0llyW00dzZ · 2024-01-04T20:45:09Z

What type of PR is this?

[+] fix(signals.go): add build tags for non-windows systems
[+] fix(signals_test.go): add build tags for non-windows systems
[+] feat(signals_windows.go): add build tags for windows systems
[+] feat(signals_windows_test.go): add build tags for windows systems

What this PR does / why we need it:

syscall.SIGTSTP is not available on Windows.

Which issue(s) this PR fixes:

Fixes #1149

Special notes for your reviewer:

build specific tags

Does this PR introduce a user-facing change?

Only build specific tags

- [+] fix(signals.go): add build tags for non-windows systems - [+] fix(signals_test.go): add build tags for non-windows systems - [+] feat(signals_windows.go): add build tags for windows systems - [+] feat(signals_windows_test.go): add build tags for windows systems

linux-foundation-easycla · 2024-01-04T20:45:16Z

The committers listed above are authorized under a signed CLA.

✅ login: H0llyW00dzZ / name: H0llyW00dzZ (67787f6, 667a209, c601f92, 1a578c9, dccf08c, ef0b26c, 322bada, b66ad0f, fd344e4, a48f2f0, 65feb8b, ecfffa1, 81ca459, 2c817bf, f3e8576)

k8s-ci-robot · 2024-01-04T20:45:18Z

Welcome @H0llyW00dzZ!

It looks like this is your first PR to kubernetes-sigs/promo-tools 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/promo-tools has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-01-04T20:45:19Z

Hi @H0llyW00dzZ. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

- [+] refactor(signals_windows_test.go): Rename file from singals_windows_test.go to signals_windows_test.go

k8s-ci-robot · 2024-01-05T09:20:56Z

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

- [+] fix(signals.go): add build tags for non-windows systems - [+] fix(signals_test.go): add build tags for non-windows systems - [+] feat(signals_windows.go): add build tags for windows systems - [+] feat(signals_windows_test.go): add build tags for windows systems

- [+] refactor(signals_windows_test.go): Rename file from singals_windows_test.go to signals_windows_test.go

- [+] chore(pr.go): Add note about complexity and suggest splitting into smaller functions - [+] chore(signals.go): Update note to specify that it builds on Unix/Linux systems - [+] chore(signals_test.go): Update note to specify that it builds on Unix/Linux systems

…to patch

cpanato

some initial comments, also will need to check if need to change the build process to adjust for this

/ok-to-test

/assign @saschagrunert for second review

cmd/kpromo/cmd/pr/pr.go

internal/legacy/signals/signals_windows.go

internal/legacy/signals/signals_windows_test.go

- [+] chore(pr.go): Update note about complexity and suggest splitting into smaller functions

- [+] chore(signals): update copyright year to 2024 in signals_windows.go and signals_windows_test.go files

- [+] test(signals_windows_test.go): add comments to explain why certain signals are not tested

- [+] refactor(pr.go): split runPromote function into smaller functions for better readability and maintainability

H0llyW00dzZ · 2024-01-05T14:43:02Z

/retest

H0llyW00dzZ · 2024-01-05T14:52:09Z

Note

boilerplate might need to changed as well

./hack/verify-boilerplate.sh
4 files have incorrect boilerplate headers:
internal/legacy/signals/signals.go
internal/legacy/signals/signals_test.go
internal/legacy/signals/signals_windows.go
internal/legacy/signals/signals_windows_test.go
make: *** [Makefile:146: verify-boilerplate] Error 1

because this correct

//go:build !windows
// +build !windows

// Note: this build on unix/linux systems

/*
Copyright 2021 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

about

//go:build !windows
// +build !windows

must be on the top

cpanato · 2024-01-19T10:04:40Z

we have other files in other repos that the go build is not in the top and works with no problem

cpanato · 2024-01-19T10:04:58Z

cc @puerco @saschagrunert @xmudrii

internal/legacy/signals/signals.go

internal/legacy/signals/signals_test.go

internal/legacy/signals/signals_windows.go

cmd/kpromo/cmd/pr/pr.go

internal/legacy/signals/signals_windows_test.go

- [+] test(signals_windows_test.go): remove unnecessary comments and update comment about unavailable signals on Windows

- [+] chore(signals_windows.go): remove commented out code for SIGTSTP signal

- [+] test(signals_windows_test.go): comment out unused signal tests - [+] test(signals_windows_test.go): comment out unused signal tests

H0llyW00dzZ · 2024-01-20T01:32:09Z

/retest

- [+] chore(pr.go): update comment to reference issue kubernetes-sigs#1177 and clarify code readability concerns

k8s-ci-robot · 2024-01-20T01:42:04Z

@H0llyW00dzZ: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cip-verify	`f3e8576`	link	true	`/test pull-cip-verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

cpanato · 2024-01-20T10:04:51Z

cmd/kpromo/cmd/pr/pr.go

@@ -158,6 +158,9 @@ func init() {
 	}
 }

+// Issue #1177: This function is overly complex and difficult to read, both for humans and machines.


add the full like and drop this comment

cpanato · 2024-01-20T10:07:09Z

i will check the boilerplate issue

cpanato · 2024-01-20T10:08:02Z

and I would like to get evidence the current build still works and also it working for windows as well

H0llyW00dzZ · 2024-01-20T17:29:11Z

and I would like to get evidence the current build still works and also it working for windows as well

a issue its only signal that not available on windows

k8s-triage-robot · 2024-04-19T17:54:53Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

xmudrii · 2024-04-22T09:38:05Z

/remove-lifecycle stale

k8s-triage-robot · 2024-07-21T10:28:52Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2024-08-20T11:22:23Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2024-09-19T11:40:20Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2024-09-19T11:40:25Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 4, 2024

k8s-ci-robot added the area/artifacts Issues or PRs related to the hosting of release artifacts for subprojects label Jan 4, 2024

k8s-ci-robot requested review from cpanato and salaxander January 4, 2024 20:45

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 4, 2024

H0llyW00dzZ mentioned this pull request Jan 4, 2024

Bug installation on windows #1149

Open

H0llyW00dzZ and others added 2 commits January 5, 2024 04:00

Fix Typo

667a209

- [+] refactor(signals_windows_test.go): Rename file from singals_windows_test.go to signals_windows_test.go

Merge branch 'kubernetes-sigs:main' into patch

c601f92

k8s-ci-robot added the do-not-merge/contains-merge-commits label Jan 5, 2024

H0llyW00dzZ added 4 commits January 5, 2024 16:23

Fix Typo

dccf08c

- [+] refactor(signals_windows_test.go): Rename file from singals_windows_test.go to signals_windows_test.go

Merge branch 'patch' of https://github.com/H0llyW00dzZ/promo-tools in…

322bada

…to patch

cpanato requested changes Jan 5, 2024

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 5, 2024

k8s-ci-robot assigned cpanato Jan 5, 2024

H0llyW00dzZ added 2 commits January 5, 2024 21:26

Chore [PR] [Signals] Update Note

b66ad0f

- [+] chore(pr.go): Update note about complexity and suggest splitting into smaller functions

Chore [Internal] [Signal] Update Copyright Year

fd344e4

- [+] chore(signals): update copyright year to 2024 in signals_windows.go and signals_windows_test.go files

H0llyW00dzZ added 2 commits January 5, 2024 21:34

Chore [Internal] [Signal Test] Update Note

a48f2f0

- [+] test(signals_windows_test.go): add comments to explain why certain signals are not tested

Chore [CMD] [Kpromo] [CMD] [PR] Add Author Note

65feb8b

- [+] refactor(pr.go): split runPromote function into smaller functions for better readability and maintainability

xmudrii requested changes Jan 19, 2024

View reviewed changes

k8s-ci-robot assigned xmudrii Jan 19, 2024

H0llyW00dzZ added 3 commits January 20, 2024 08:21

Update Note

ecfffa1

- [+] test(signals_windows_test.go): remove unnecessary comments and update comment about unavailable signals on Windows

Update Note & remove commented out code

81ca459

- [+] chore(signals_windows.go): remove commented out code for SIGTSTP signal

Remove Comment Code

2c817bf

- [+] test(signals_windows_test.go): comment out unused signal tests - [+] test(signals_windows_test.go): comment out unused signal tests

Update Comment

f3e8576

- [+] chore(pr.go): update comment to reference issue kubernetes-sigs#1177 and clarify code readability concerns

cpanato reviewed Jan 20, 2024

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 19, 2024

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 22, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 21, 2024

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 20, 2024

k8s-ci-robot closed this Sep 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Signal Issue on Windows #1159

Fix Signal Issue on Windows #1159

H0llyW00dzZ commented Jan 4, 2024

linux-foundation-easycla bot commented Jan 4, 2024 •

edited

Loading

k8s-ci-robot commented Jan 4, 2024

k8s-ci-robot commented Jan 4, 2024

k8s-ci-robot commented Jan 5, 2024

cpanato left a comment

H0llyW00dzZ commented Jan 5, 2024

H0llyW00dzZ commented Jan 5, 2024

cpanato commented Jan 19, 2024

cpanato commented Jan 19, 2024

H0llyW00dzZ commented Jan 20, 2024

k8s-ci-robot commented Jan 20, 2024

cpanato Jan 20, 2024

cpanato commented Jan 20, 2024

cpanato commented Jan 20, 2024

H0llyW00dzZ commented Jan 20, 2024

k8s-triage-robot commented Apr 19, 2024

xmudrii commented Apr 22, 2024

k8s-triage-robot commented Jul 21, 2024

k8s-triage-robot commented Aug 20, 2024

k8s-triage-robot commented Sep 19, 2024

k8s-ci-robot commented Sep 19, 2024

Fix Signal Issue on Windows #1159

Fix Signal Issue on Windows #1159

Conversation

H0llyW00dzZ commented Jan 4, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

linux-foundation-easycla bot commented Jan 4, 2024 • edited Loading

k8s-ci-robot commented Jan 4, 2024

k8s-ci-robot commented Jan 4, 2024

k8s-ci-robot commented Jan 5, 2024

cpanato left a comment

Choose a reason for hiding this comment

H0llyW00dzZ commented Jan 5, 2024

H0llyW00dzZ commented Jan 5, 2024

cpanato commented Jan 19, 2024

cpanato commented Jan 19, 2024

H0llyW00dzZ commented Jan 20, 2024

k8s-ci-robot commented Jan 20, 2024

cpanato Jan 20, 2024

Choose a reason for hiding this comment

cpanato commented Jan 20, 2024

cpanato commented Jan 20, 2024

H0llyW00dzZ commented Jan 20, 2024

k8s-triage-robot commented Apr 19, 2024

xmudrii commented Apr 22, 2024

k8s-triage-robot commented Jul 21, 2024

k8s-triage-robot commented Aug 20, 2024

k8s-triage-robot commented Sep 19, 2024

k8s-ci-robot commented Sep 19, 2024

linux-foundation-easycla bot commented Jan 4, 2024 •

edited

Loading