I’m currently working on adding support in Change Request to page refactoring to ensure that if you opened a Change Request for page Foo and that page is moved to Bar, then the Change Request references properly the page Bar.
The work is almost done but I have questions related to how it should work:
Do you think it should also refactor closed change requests (including merged ones)?
Do you think reviews should be invalidated once a change request has been refactored automatically?
For 1, my first move would be to only perform the refactoring on opened change requests only: I’m worried that CR keeps piling up over time without any deletion and that the refactoring of pages becomes more and more expensive. Now, I already discussed with users and I’m already planning to provide a configuration to allow refactoring of closed change request, the question is more what should be the default for such configuration?
For 2, I actually wanted to be consistent with what’s happening when rebasing (refreshing) a change request: right now, any operation on a CR invalidates the reviews as it might impacts the content and we want reviewers to check latest changes. So to be consistent I was thinking it would make sense that the reviews are invalidated whenever a refactoring happened on an open CR, mainly to ensure everything is still ok. Now there’s no technical need here so it’s also something that could be configurable. WDYT?
IMO, it does not make any sense to refactor merged change requests, since it’s not related to the current status anymore. For those which are just closed, I don’t know, I guess it depends on how likely it is for such change requests to be reopened one day (in which case, we’ll want it to be up-to-date), maybe it’s not possible to reopen them ?
I find it a pity to lose the exact location a review is related to just because the reference of the document changed.
Hmmm there’s a misunderstanding here I think. The review discussion are kept and refactoring, so the location of a review comment is never lost. What I’m talking about is the actual state of a review to approve or disapprove changes: there’s a flag to invalidate the approval in case of changes happening after the approval basically. That’s the invalidation I’m talking about.
OK, thanks for the correction, I was indeed a bit off in my understanding
Then indeed there is no reason that applying a refactoring is the only rebase operation which does not reset the validation status. To be honest, it might actually feel a bit weird to apply this external change automatically, but require a manual rebase trigger for others.
I would find it weird for closed change requests to not move with the page. Imaging you move page A to B and then create a new page A - it wouldn’t make any sense for the old change requests to show up again on A, would it? Now it is not clear to me if we would need to fully refactor the change request, to me the important part is that it is listed on the new and not on the old location. A full refactoring could possibly be delayed to the change request actually being re-opened for example.
Regarding the invalidation, it seems okay to me to invalidate the reviews but I’m not that deep into change request to understand the full implications and use cases.
