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 unit sec to delay parameter #33

Merged
merged 1 commit into from
Jun 24, 2023

Conversation

kkpattern
Copy link
Contributor

The delay parameter requires a "sec" unit to function correctly.

I also added a test for this.

The delay parameter requires a "sec" unit to function correctly.
@pbelskiy
Copy link
Owner

@kkpattern Hello! Thanks or your PR. But when Jenkins starts to require it?

Could you please share some information about this?

@kkpattern
Copy link
Contributor Author

@kkpattern Hello! Thanks or your PR. But when Jenkins starts to require it?

Could you please share some information about this?

Sorry I don’t know since when Jenkins starts to require it. We have added the sec unit since 2019 in jenkinsapi. You can easily reproduce this by removing the sec unit and run the newly added test. The build will be triggered immediately instead of waiting for two seconds.

@pbelskiy
Copy link
Owner

@kkpattern thanks! I will check it later

@pbelskiy pbelskiy merged commit 2b233e0 into pbelskiy:master Jun 24, 2023
1 of 16 checks passed
@pbelskiy
Copy link
Owner

pbelskiy commented Jun 24, 2023

@kkpattern thanks. also you could create PR for https://github.com/pbelskiy/ujenkins if you want!

PS: using f-string is preferrable

@kkpattern
Copy link
Contributor Author

@kkpattern thanks. also you could create PR for https://github.com/pbelskiy/ujenkins if you want!

PS: using f-string is preferrable

Happy to create PR for ujenkins. I didn't use f-string in aiojenkins because I remembered aiojenkins still supports python3.5 right? I will use f-string in ujenkins.

@pbelskiy
Copy link
Owner

@kkpattern actually support of Python 3.5 and 3.6 was dropped half year ago.

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.

None yet

2 participants