Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] Support specifying a pin for merges #73

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

thomaspaulb
Copy link
Contributor

@thomaspaulb thomaspaulb commented Jul 18, 2023

Depends on:

Use case:

Currently in gitaggregator, you can already do:

OCA/web:
  defaults:
    depth: 10
  remotes:
    origin: https://github.com/OCA/web.git
    forgeflow: https://github.com/forgeflow/web.git
  target: origin 16.0
  merges:
    - origin d20edd1b171c166b6f7d016ac2cfb044a95f084f
    - forgeflow 27be86ab189d91aeb9b4be7bb11198dc016f43fa
  fetch_all: true

With this change, you can specify the exact branches on which the commits are and do:

OCA/web:
  defaults:
    depth: 100
  remotes:
    origin: https://github.com/OCA/web.git
    forgeflow: https://github.com/forgeflow/web.git
  target: origin 16.0
  merges:
    - origin 16.0 d20edd1b171c166b6f7d016ac2cfb044a95f084f
    - forgeflow 16.0-mig-web_notify_channel_message 27be86ab189d91aeb9b4be7bb11198dc016f43fa

So that fetch_all = True is not needed anymore and just the branches need to be fetched, and then reset to the pinned commit.

NB: I notice that Github currently also accept querying directly for the SHA. Is that new? Eg just this also works:

OCA/web:
  defaults:
    depth: 10
  remotes:
    origin: https://github.com/OCA/web.git
    forgeflow: https://github.com/forgeflow/web.git
  target: origin 16.0
  merges:
    - origin d20edd1b171c166b6f7d016ac2cfb044a95f084f
    - forgeflow 27be86ab189d91aeb9b4be7bb11198dc016f43fa

@thomaspaulb thomaspaulb changed the title [DRAFT] Support specifying a pin for merges new: Support specifying a pin for merges Jul 18, 2023
@thomaspaulb thomaspaulb changed the title new: Support specifying a pin for merges [ADD] Support specifying a pin for merges Jul 18, 2023
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #73 (89db99c) into master (fe1e0bb) will increase coverage by 0.14%.
The diff coverage is 90.36%.

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   82.64%   82.78%   +0.14%     
==========================================
  Files           6        6              
  Lines         409      459      +50     
==========================================
+ Hits          338      380      +42     
- Misses         71       79       +8     
Impacted Files Coverage Δ
git_aggregator/repo.py 75.53% <89.74%> (+1.40%) ⬆️
git_aggregator/config.py 99.02% <100.00%> (+0.03%) ⬆️

@sbidoul
Copy link
Member

sbidoul commented Jul 22, 2023

Can you describe the use case?

@sbidoul
Copy link
Member

sbidoul commented Jul 22, 2023

Can you describe the use case?

Ah, I now see this is #50

@sbidoul
Copy link
Member

sbidoul commented Jul 22, 2023

What happens if the target branch is rebased and thus the specified commit does not exist anymore on that branch?

@thomaspaulb
Copy link
Contributor Author

What happens if the target branch is rebased and thus the specified commit does not exist anymore on that branch?

Then the build fails.

vaab and others added 3 commits July 22, 2023 23:48
…csone#28)

Avoids also contacting twice (``fetch`` and then ``pull``) remote before
merging.

Signed-off-by: Valentin Lab <[email protected]>
@thomaspaulb
Copy link
Contributor Author

Can you describe the use case?

I've updated the PR description to specify the use case, but it all becomes a bit thin if we are also allowed to query directly for the SHA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants