Warn & reset fw policy if limit is updated #953
Labels
Comments
xmo-odoo
added a commit
that referenced
this issue
Sep 24, 2024
Apparently when I added these to trigger the corresponding cron the lack of decorator caused them to not work at all, at least when invoked from the interface, consequently I notably wasn't able to force a forward port via creating one such task when trying to work around #953
xmo-odoo
added a commit
that referenced
this issue
Sep 24, 2024
The UX around the split of limit and forward port policy (and especially moving "don't forward port" to the policy) was not really considered and caused confusion for both me and devs: after having disabled forward porting, updating the limit would not restore it, but there would be no indication of such an issue. This caused odoo/enterprise#68916 to not be forward ported at merge (despite looking for all the world like it should be), and while updating the limit post-merge did force a forward-port that inconsistency was just as jarring (also not helped by being unable to create an fw batch via the backend UI because reasons, see 10ca096). Fix this along most axis: - Notify the user and reset the fw policy if the limit is updated while `fw=no`. - Trigger a forward port if the fw policy is updated (from `no`) on a merged PR, currently only sources. - Add check & warning to limit update process so it does *not* force a port (though maybe it should under the assumption that we're updating the limit anyway? We'll see). Fixes #953
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
odoo/enterprise#68916 got very confusing as after disabling forwardport the limit was reset yet it was not forwardported.
The answer is that since 7711d09 these are now two completely different items, and the limit is updated independently from the forward port policy (because PRs of the same batch can have different limits).
So when the limit was updated, it had no effect as the limit was already False ~ master. Thankfully the post-merge resumption did not check the forward-port policy, so went through.
_maybe_update_limit, currently it unconditionally creates a forward-port task if the tip of the chain is merged (e.g. if the tip of the chain is the source and forward porting was disabled) which is inconsistent: if the tip is not merged then it just schedules a followup which does check for the policynotodefaultafter merge, the same wayup todoes.fw=defaultcan be invoked by the PR author to reset fromfw=noThe text was updated successfully, but these errors were encountered: