search

cms-gem-daq-project / ctp7_modules

0 stars 13 forks source link

Merge templated RPC migration into develop #176

Closed lpetre-ulb closed 4 years ago

lpetre-ulb commented 4 years ago

Description

Following what was done in xHAL and as described in https://github.com/cms-gem-daq-project/ctp7_modules/issues/172, this PR aims at making develop the main development branch.

The only active development is around the templated RPC, but the target branch is different between different repositories (develop in xhal and cmsgemos, but feature/templated-rpc in this repository). It is somehow confusing.

The develop branch of cmsgemos is going to start using the templated RPC framework, so it makes sense to include the templated RPC functionalities into develop.

Moreover, some of the latest clean up PR would be target develop rather than feature/templated-rpc-methods, but are (going to be) merged into feature/templated-rpc-methods to avoid any divergences.

Since fast forward merge is better done from the command line (committers are preserved), I'm going to push the change this evening if there is no strong opposition.

Types of changes

Motivation and Context

The templated RPC are the present of the CTP7 modules.

jsturdy commented 4 years ago

Historically, for all merges done via PR/MR (except for those in throwaway branches, e.g., what was supposed to be the "short-term" legacy code) should have a merge commit to mark the PR/MR through which the merge happened (history and automatic changelog are two of many reasons to do this). As a corollary, no other PR/MRs should appear in the "central" tree, i.e., if people do PR/MRs between their developments (which has been discouraged, but has still happened), those should be squash, fast-forward, rebase, whatever, but without a merge commit

lpetre-ulb commented 4 years ago

Historically, for all merges done via PR/MR (except for those in throwaway branches, e.g., what was supposed to be the "short-term" legacy code) should have a merge commit to mark the PR/MR through which the merge happened (history and automatic changelog are two of many reasons to do this).

It makes a lot sense for regular PR/MR. Does it also make so much sense for central branches merges? Particularly in this case where the development was stalled in develop and only occured in feature/templated-rpc-methods? I don't have a strong opinion about it. This PR can be merged as-is if you prefer.

However, that rule has been very loosely applied. If I'm not mistaken, non fast-forward merges are even forbidden in some repositories, or, at least, not the default option.

As a corollary, no other PR/MRs should appear in the "central" tree, i.e., if people do PR/MRs between their developments (which has been discouraged, but has still happened), those should be squash, fast-forward, rebase, whatever, but without a merge commit

:+1: :+1: :+1:

mexanick commented 4 years ago

Merging as is will allow to loosely keep the commit history after deletion of the templated-rpc-methods branch deletion which I believe is preferred way in current circumstances. Otherwise, squash commit can be applied, but then history tracking will be difficult and would requires preserving of then-obsolete templated-rpc-methods branch