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.

Sign up for GitHub

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

Jump to bottom

Add 'reason' field in the lock() step. #426

Open
mPokornyETM opened this issue Dec 7, 2022 · 8 comments · May be fixed by #520
Open

Add 'reason' field in the lock() step. #426

mPokornyETM opened this issue Dec 7, 2022 · 8 comments · May be fixed by #520
Labels
enhancement extended properties Ideas about more aspects of the lockable resource instances than we had before good first issue Good for newcomers Need test Automated tests shall be extended. new parameters/features ui Features that may impact UI, pages made by the plugin or external UIs (BO, legacy, etc.)
Milestone
Feature committed

Comments

@mPokornyETM
Copy link
Contributor

What feature do you want to see added?

It will be very helpful, when we the lock() step allow to set the reason.
At the moment we can see that the resource is locked by some job. But when you have parallel stages it is hard to see which stage / node ... lock the resource.
In this case shall be this text shown in extra column in the table (or append to the STATE as well)
The text shall be pure String. No markUp or html format is necessary.

My first think was to use note as well, but this will leads to misunderstandings.

Upstream changes

No response
@mPokornyETM
Copy link
Contributor Author

see also JENKINS-68225

@mPokornyETM
Copy link
Contributor Author

It will be fine when user can type the reason also on reserve action in the overview page. And maybe we can use the same field from config page. There is reserved by field, but you can type anything there.

@mPokornyETM mPokornyETM added the Triage Need to clarify, remove, close or whatever to clean up open issues / PRs label Feb 1, 2023
@mPokornyETM mPokornyETM added good first issue Good for newcomers ui Features that may impact UI, pages made by the plugin or external UIs (BO, legacy, etc.) extended properties Ideas about more aspects of the lockable resource instances than we had before new parameters/features Need test Automated tests shall be extended. and removed Triage Need to clarify, remove, close or whatever to clean up open issues / PRs labels Feb 6, 2023
@Massakera
Copy link

Hii @mPokornyETM! Are this feature still needed? I would love to work on that

@mPokornyETM
Copy link
Contributor Author

mPokornyETM commented Jun 12, 2023 •
edited
Loading

@Massakera Hi, all open issues / PRs with the milestone https://github.com/jenkinsci/lockable-resources-plugin/milestone/5 are planned and 'needed'
It will be grat, when somebody (maybe you ;-) ) will implemented this request.
PS: any contributions are welcome. See also good first issue Good for newcomers

@Massakera
Copy link

Massakera commented Jun 12, 2023 •
edited
Loading

I think I've added the reason field, but still I can't see it the field when adding a lockable resource when I run Jenkins locally with mvn hpi:run. Can you see if I'm doing in the right way? Massakera@d76c533

@mPokornyETM
Copy link
Contributor Author

Yu need also to extend:

do not scary, try it in the order as described and you will see it goes straigth forwards

@Massakera
Copy link

Massakera commented Jun 13, 2023 •
edited
Loading

image

So, I was able to made the changes and I can put the reason field when setting up the Lockable resource in Settings page, but for some reason the labels shifts to Reason column when I get to the Lockable Resources page and I don't know why.

This is my tableResources/table.jelly

@mPokornyETM
Copy link
Contributor Author

please create a draft PR, that I can see your changes.
thx

@Massakera Massakera linked a pull request Jun 13, 2023 that will close this issue
Draft
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement extended properties Ideas about more aspects of the lockable resource instances than we had before good first issue Good for newcomers Need test Automated tests shall be extended. new parameters/features ui Features that may impact UI, pages made by the plugin or external UIs (BO, legacy, etc.)
Projects
None yet
Milestone
Feature committed
Development

Successfully merging a pull request may close this issue.

Add 'reason' field in the lock() step Massakera/lockable-resources-plugin
2 participants
@Massakera @mPokornyETM