Hi!
I've noticed a certain problem in our workflow that I'd like to discuss.
From time to time, we deprecate certain APIs in core or extensions. The
idea of deprecation is that we have gradual transition - existing code keeps working, and we gradually switch to the new API over time, instead of removing old API at one sweep and breaking all the existing code. This is the theory, and it is solid.
Also, we have phan checks on the CI builds, which prevent us from using deprecated APIs. Right now if phan detects deprecated API, it fails the build.
Now, combining this produces a problem - if somebody deprecates an API that I use anywhere in my extension (and some extensions can be rather big and use a lot of different APIs), all patches to that extension immediately have their builds broken - including all those patches that have nothing to do with the changed API. Of course, the easy way is just to add @suppress and phan shuts up - but that means, the deprecated function is completely off the radar, and nobody probably would remember to look at it again ever.
I see several issues here with this workflow: 1. Deprecation breaks unrelated patches, so I have no choice but to shut it up if I want to continue my work. 2. It trains me that the reaction to phan issues should be to add @suppress - which is the opposite of what we want to happen. 3. The process kind of violates the spirit of what deprecation is about - existing code doesn't keep working without change, at least as far as the build is concerned, and constantly @suppress-ing phan diminishes the value of having those checks in the first place.
I am not sure what would be the best way to solve this, so I'd like to hear some thoughts on this from people. Do you also think it is a problem or it's just me? What would be the best way to improve it?
Thanks,
I'm going to plug my favorite tool: https://www.sonarqube.org
SonarQube can have rules about breaking builds that are a bit more complex than most tools. In particular, it can fail only if violations happen on code you actually touched. In this specific case, it means you could have a set of rules that would allow new deprecation to happen. If you touch a piece of code that is using a deprecated API, then you have to fix it.
I'll try to find a good intro to SonarQube and I'll come back with a link...
On Tue, Dec 19, 2017 at 7:01 PM, Stas Malyshev smalyshev@wikimedia.org wrote:
Hi!
I've noticed a certain problem in our workflow that I'd like to discuss.
From time to time, we deprecate certain APIs in core or extensions. The idea of deprecation is that we have gradual transition - existing code keeps working, and we gradually switch to the new API over time, instead of removing old API at one sweep and breaking all the existing code. This is the theory, and it is solid.
Also, we have phan checks on the CI builds, which prevent us from using deprecated APIs. Right now if phan detects deprecated API, it fails the build.
Now, combining this produces a problem - if somebody deprecates an API that I use anywhere in my extension (and some extensions can be rather big and use a lot of different APIs), all patches to that extension immediately have their builds broken - including all those patches that have nothing to do with the changed API. Of course, the easy way is just to add @suppress and phan shuts up - but that means, the deprecated function is completely off the radar, and nobody probably would remember to look at it again ever.
I see several issues here with this workflow:
- Deprecation breaks unrelated patches, so I have no choice but to shut
it up if I want to continue my work. 2. It trains me that the reaction to phan issues should be to add @suppress - which is the opposite of what we want to happen. 3. The process kind of violates the spirit of what deprecation is about
- existing code doesn't keep working without change, at least as far as
the build is concerned, and constantly @suppress-ing phan diminishes the value of having those checks in the first place.
I am not sure what would be the best way to solve this, so I'd like to hear some thoughts on this from people. Do you also think it is a problem or it's just me? What would be the best way to improve it?
Thanks,
Stas Malyshev smalyshev@wikimedia.org
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I’m not entirely familiar with how we use Phan, but if it’s like the other tests we run with Gerrit, what we could do is have a test for deprecated APIs that passes or fails as you might expect, but have it be a non-voting test. This way a failure is a signal to check something out, but it doesn’t stop the build altogether.
On Dec 19, 2017, at 10:01 AM, Stas Malyshev smalyshev@wikimedia.org wrote:
Hi!
I've noticed a certain problem in our workflow that I'd like to discuss.
From time to time, we deprecate certain APIs in core or extensions. The idea of deprecation is that we have gradual transition - existing code keeps working, and we gradually switch to the new API over time, instead of removing old API at one sweep and breaking all the existing code. This is the theory, and it is solid.
Also, we have phan checks on the CI builds, which prevent us from using deprecated APIs. Right now if phan detects deprecated API, it fails the build.
Now, combining this produces a problem - if somebody deprecates an API that I use anywhere in my extension (and some extensions can be rather big and use a lot of different APIs), all patches to that extension immediately have their builds broken - including all those patches that have nothing to do with the changed API. Of course, the easy way is just to add @suppress and phan shuts up - but that means, the deprecated function is completely off the radar, and nobody probably would remember to look at it again ever.
I see several issues here with this workflow:
- Deprecation breaks unrelated patches, so I have no choice but to shut
it up if I want to continue my work. 2. It trains me that the reaction to phan issues should be to add @suppress - which is the opposite of what we want to happen. 3. The process kind of violates the spirit of what deprecation is about
- existing code doesn't keep working without change, at least as far as
the build is concerned, and constantly @suppress-ing phan diminishes the value of having those checks in the first place.
I am not sure what would be the best way to solve this, so I'd like to hear some thoughts on this from people. Do you also think it is a problem or it's just me? What would be the best way to improve it?
Thanks,
Stas Malyshev smalyshev@wikimedia.org
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I agree with this. Splitting the deprecation phan issues out to a separate non-voting job sounds like a good idea (and should be easily supported by phan with its issue whitelist/blacklist config directive)
Ideally we would have a phan plugin that generates warnings for hard deprecations (wfDeprecated()), so that those could be voting.
-- brian
On Tuesday, December 19, 2017, James Hare jamesmhare@gmail.com wrote:
I’m not entirely familiar with how we use Phan, but if it’s like the
other tests we run with Gerrit, what we could do is have a test for deprecated APIs that passes or fails as you might expect, but have it be a non-voting test. This way a failure is a signal to check something out, but it doesn’t stop the build altogether.
On Dec 19, 2017, at 10:01 AM, Stas Malyshev smalyshev@wikimedia.org
wrote:
Hi!
I've noticed a certain problem in our workflow that I'd like to discuss.
From time to time, we deprecate certain APIs in core or extensions. The idea of deprecation is that we have gradual transition - existing code keeps working, and we gradually switch to the new API over time, instead of removing old API at one sweep and breaking all the existing code. This is the theory, and it is solid.
Also, we have phan checks on the CI builds, which prevent us from using deprecated APIs. Right now if phan detects deprecated API, it fails the build.
Now, combining this produces a problem - if somebody deprecates an API that I use anywhere in my extension (and some extensions can be rather big and use a lot of different APIs), all patches to that extension immediately have their builds broken - including all those patches that have nothing to do with the changed API. Of course, the easy way is just to add @suppress and phan shuts up - but that means, the deprecated function is completely off the radar, and nobody probably would remember to look at it again ever.
I see several issues here with this workflow:
- Deprecation breaks unrelated patches, so I have no choice but to shut
it up if I want to continue my work. 2. It trains me that the reaction to phan issues should be to add @suppress - which is the opposite of what we want to happen. 3. The process kind of violates the spirit of what deprecation is about
- existing code doesn't keep working without change, at least as far as
the build is concerned, and constantly @suppress-ing phan diminishes the value of having those checks in the first place.
I am not sure what would be the best way to solve this, so I'd like to hear some thoughts on this from people. Do you also think it is a problem or it's just me? What would be the best way to improve it?
Thanks,
Stas Malyshev smalyshev@wikimedia.org
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
wikitech-l@lists.wikimedia.org