workflow-support-plugin icon indicating copy to clipboard operation
workflow-support-plugin copied to clipboard

Move `SemaphoreStep` states into a single map to try to simplify the implementation

Open dwnusbaum opened this issue 10 months ago • 1 comments

See https://github.com/jenkinsci/workflow-support-plugin/pull/258#issuecomment-2026171643.

Personally, I feel like this is still pretty comparable to https://github.com/jenkinsci/workflow-support-plugin/pull/258 in terms of complexity. It seems a little better in terms of synchronization complexity, but the state transition complexity seems about the same or worse. The issue is that the four maps did not represent distinct non-overlapping states and so it is still not that clear what is going on in the new code. If we are purely going for simplicity, something like https://github.com/jenkinsci/workflow-support-plugin/pull/259 would be my vote, then #258, then this PR.

If we want to go with this approach, I think this PR would need a PCT run because it seems possible that is accidentally broke some typical use case.

dwnusbaum avatar Mar 28 '24 22:03 dwnusbaum

this PR would need a PCT run

We should do a PCT run for any proposed change to this class. (jenkinsci/bom and/or @cloudbees internal)

jglick avatar Mar 29 '24 12:03 jglick

I have no plans to finish this up, so I am closing it; the two tests mentioned above in other plugins would require changes either here or to the tests depending on the desired behavior.

dwnusbaum avatar Aug 13 '24 21:08 dwnusbaum