Remove package.json related code
Closed this issue · 8 comments
package.json never happened.
Tracking: https://github.com/mozilla/addons-server/issues/2460
Hm, we need another reliable way to identify SDK based add-ons, then.
We do have https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L42 and given that cfx-based add-ons don't work in any supported firefox version (including esr which is based on fx45) I think we could remove that part and assume that all jetpack add-ons are the same again?
Where is inspect_jetpack (https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L42) called?
Also, inside that method, in https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L48 we call https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L165 which relies on package.json.
inspect_jetpack should be called automatically when all tier 1 checks are being run. Don't ask me where that is specifically though, somewhere is a registry for that.
My idea / question was wether or not we could get rid of those lines: https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L55-L70 and with them remove has_package_json and adapt https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L165-L171 - so all jetpack add-ons are "new-style" again. Does that even make sense?
I don't think we should remove https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L55-L70 just yet. Add-ons built using cfx might still be uploaded. Also, I don't see how those lines are the core of the problem?
If what you say is right and inspect_jetpack is called for all add-ons, we need has_package_json for the check in https://github.com/mozilla/amo-validator/blob/master/validator/testcases/jetpack.py#L48. How would we mitigate that?
Everything I thought was based on the idea that cfx based add-ons aren't a thing anymore, if they are then we can't remove package.json just yet and have to wait.
If they weren't a thing anymore, we could remove is_jetpack and rename is_old_jetpack to is_jetpack since there would be only one jetpack format again.
But again, since that's not the case we should probably wait. It doesn't hurt, and the problems we had were fixed directly in addons-server.
I'm closing this then for now as this probably won't happen too soon if there are still is a chance of cfx add-ons being uploaded. And, it doesn't hurt :)
Quick note, this ticket was mostly about code-cleanup and is kinda unrelated to cfx vs jpm. The issues we had were fixed in addons-server but the validator continues to lint and extract package.json which is fine.