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

[JENKINS-26466] Decouple AbstractProject from DiskSpaceMonitor #1983

Conversation

batmat
Copy link
Member

@batmat batmat commented Jan 15, 2016

This modification is one of the preliminary steps to the full plugin extraction later (see JIRA above).
Basically reverts a8c3a03.

This modification is one of the preliminary steps to the full plugin
extraction later (see JIRA above).
Basically reverts a8c3a03.
@daniel-beck
Copy link
Member

👍

@batmat
Copy link
Member Author

batmat commented Jan 16, 2016

For reference, I first wrote another version but as advised by @jglick I finally went on the leaner removal version.

@oleg-nenashev
Copy link
Member

I have no strong opinion. Are you planning to somehow implement it in the decoupled plugin?
Also CC @ndeloof

@jtnord
Copy link
Member

jtnord commented Jan 17, 2016

👍

@batmat
Copy link
Member Author

batmat commented Jan 17, 2016

@oleg-nenashev No I didn't plan to currently, but if people think it's useful I can absolutely consider doing so. That may require introducing another new extension point like the one in #1985

It would also indeed be useful to have a word of @ndeloof about the use case that led him to add this code at the time. If he still thinks this should be kept at least in the short term, I can file a new JIRA and will consider reintroducing that.

@olivergondza
Copy link
Member

The use-case, as I understand it, is that some part of core suspect the slave can be out of disk space and should have a was to ask monitoring subsystem to have the slave rechecked and turned offline eventualy. There is lot more places like that in core - not to mention this is not specific to diskspace monitoring. For some time I wanted to refactor the code so this will be possible for all the monitors, but never actually had time to.

👍

@oleg-nenashev oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jan 20, 2016
@oleg-nenashev
Copy link
Member

ready to go, because there was no votes against the change

@batmat
Copy link
Member Author

batmat commented Jan 27, 2016

Ping @oleg-nenashev anything still missing?

olivergondza added a commit that referenced this pull request Jan 30, 2016
…roject-from-DiskSpaceMonitor

[JENKINS-26466] Decouple AbstractProject from DiskSpaceMonitor
@olivergondza olivergondza merged commit 59540fa into jenkinsci:master Jan 30, 2016
@batmat
Copy link
Member Author

batmat commented Jan 30, 2016

Thanks Oliver.
Le 30 janv. 2016 7:05 AM, "Oliver Gondža" notifications@github.com a
écrit :

Merged #1983 #1983.


Reply to this email directly or view it on GitHub
#1983 (comment).

@batmat batmat deleted the JENKINS-26466-decouple-AbstractProject-from-DiskSpaceMonitor branch November 14, 2017 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
5 participants