Electron-Ion Collider (EIC) Software
Replies: 0 comments 11 replies
-
|
This would indeed be very useful, and that's a good starting point (as anything from acts). My only concern is that forks don't have access to repository secrets needed to start benchmarks outside of GitHub. We can work around that but it's significant work. |
Beta Was this translation helpful? Give feedback.
-
|
So you run git merge in your pipeline, if it fails you don't have to run the build. If we setup actions that will rebase user branches, that would have to face the same conflicts but during the rebase. |
Beta Was this translation helpful? Give feedback.
-
|
Yes, there are indeed multiple ways to deal with this. I was just pointing out the reason for why we require geometry branches to be current against the target branch before they can be merged. That's the solution that we have currently implemented in the geometry repositories. EICrecon can do what they want. |
Beta Was this translation helpful? Give feedback.
-
|
I see, so it's a simple solution that can be already achieved with what GitHub provides. There is a downside to that, as it was mentioned on the Mattermost, requiring branches to be up to date with |
Beta Was this translation helpful? Give feedback.
-
|
Yes, there's a bandwidth issue, but we don't hit it right now (we'd need 24 PRs per day or so). That's the same with any solution, since even if you merge at the start of the pipeline you are expecting the target branch to remain unchanged until the pipeline succeeds. The only thing that can improve this is faster pipelines (which we are also working on). There is however a maximum theoretical bandwidth given pipeline duration and there is the practical matter of someone being able to either rebase/merge target into branch or resubmit a pipeline which will use the newer target as soon a new commit lands there. That's currently a manual operation. That's what e.g. eic/epic#102 aims to automate for pull requests that are already set to auto-merge. |
Beta Was this translation helpful? Give feedback.
-
|
Hopefully, this feature will become public soon https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request-with-a-merge-queue |
Beta Was this translation helpful? Give feedback.
-
As was mentioned on the last meeting our git workflow needs improvement.
I would propose to choose a policy on what is the recommended way of working with EICRecon repository.
The next workflow recommendations is an example from ACTS:
Workflow recommendations
In the following a few recommendations are outlined which should help you to get familiar with development process in the Acts project.
Each development in its own branch of your private fork!
Write access for developers has been disabled for developers on acts-project. Therefore, always start by creating your own fork and
creating branches there. You should start a new branch for every development and all work which is logically/conceptually linked
should happen in one branch. Try to keep your branches short. This helps immensely to understand the git history if you need to look at
it in the future and helps reviewers of your code. If projects are complex (e.g. large code refactoring or complex new features), you may want to use sub-branches from the main development branch.
Never, ever directly work on any "official" branch!
Though not strictly necessary and in the end it is up to you, it is strongly recommended that you never commit directly on a branch which tracks an "official" branch. As all branches are equal in git, the definition of "official" branch is quite subjective. In the Acts project you should not work directly on branches which are protected in the repository. Usually, these are the main and release/X.Y.Z branches. The benefit of this strategy is that you will never have problems to update your fork. Any git merge in your local repository on such an "official" branch will always be a fast-forward merge.
Use atomic commits!
Similarly to the concept of branches, each commit should reflect a self-contained change. Try to avoid overly large commits (bad examples are for instance mixing logical change with code cleanup and typo fixes).
Write good commit messages!
Well-written commit messages are key to understand your changes. There are many guidelines available on how to write proper commit logs (e.g. here here here)
As a short summary:
is best achieved by avoiding the
commit -moption. Instead write the commit message in an editor/git tool/IDE...Prefer git pull --rebase!
If you work with a colleague on a new development, you may want to include his latest changes. This is usually done by calling
git pullwhich will synchronise your local working copy with the remote repository (which may have been updated by your colleague). By default, this action creates a merge commit if you have local commits which were not yet published to the remote repository. These merge commits are considered to contribute little information to the development process of the feature and they clutter the history (read more e.g.here or here](http://victorlin.me/posts/2013/09/30/keep-a-readable-git-history).
This problem can be avoided by using
git pull --rebasewhich replays your local (un-pushed) commits on the tip of the remote branch. You can make this the default behaviour by runninggit config pull.rebase true. More about merging vs rebasing can be found hereUpdate the documentation!
Make sure that the documentation is still valid after your changes. Perform updates where needed and ensure integrity between the code and its documentation.
Beta Was this translation helpful? Give feedback.
All reactions