Skip to content

fix: always add branch in gh pages changes#450

Merged
kairoaraujo merged 3 commits into
eclipse-csi:mainfrom
kairoaraujo:fix-245
Jun 12, 2025
Merged

fix: always add branch in gh pages changes#450
kairoaraujo merged 3 commits into
eclipse-csi:mainfrom
kairoaraujo:fix-245

Conversation

@kairoaraujo

Copy link
Copy Markdown
Contributor

applying changes to gh pages always requires the source branch in the payload.

this change make sure that source branch is always added

closes #245

@netomi

netomi commented Jun 4, 2025

Copy link
Copy Markdown
Member

The PR looks good, however as I noted, in order to get rid of these special cases, one could apply the expected configuration all the time a change is detected, see for example repo secrets (where you are always forced to pass the secret value):

https://github.com/eclipse-csi/otterdog/blob/main/otterdog/models/repo_secret.py#L67

I was starting with the logic to have only a minimal set of changes to be applied, but the GH API has some quirks like this case, and you we can discuss whether it is worth to always apply workarounds this these cases, or just accept that in some rare cases unnecessary update calls might get triggered.

Comment thread otterdog/models/repository.py Outdated
mbarbero
mbarbero previously approved these changes Jun 4, 2025
@netomi

netomi commented Jun 4, 2025

Copy link
Copy Markdown
Member

so I would be careful to make a similar fix in 2 different places now, so I would also refactor the existing hack to the same place imho.

@kairoaraujo

Copy link
Copy Markdown
Contributor Author

so I would be careful to make a similar fix in 2 different places now, so I would also refactor the existing hack to the same place imho.

I'm moving them to this block in other to avoid diff with unchanged items.

applying changes to gh pages always requires the source branch in the
payload.

this change make sure that source branch is always added

Signed-off-by: Kairo Araujo <kairo@dearaujo.nl>
mbarbero
mbarbero previously approved these changes Jun 4, 2025
@kairoaraujo
kairoaraujo requested a review from netomi June 4, 2025 14:01
Comment thread otterdog/models/repository.py Outdated
@kairoaraujo

Copy link
Copy Markdown
Contributor Author

Hi @netomi,

Let me know if this approach looks better. If it looks ok, I will them

  1. add unit tests
  2. implement the same (in another PR) for the squash commit settings

netomi
netomi previously approved these changes Jun 5, 2025
Signed-off-by: Kairo Araujo <kairo@dearaujo.nl>
Signed-off-by: Kairo Araujo <kairo@dearaujo.nl>
@kairoaraujo

Copy link
Copy Markdown
Contributor Author

Hi @netomi and @mbarbero

I added UT, actually with UT I found a small bug that is fixed.

While adding the UT I did a little refactoring for using pytest fixture, so I can use the pytest.mark.parametrize and we will be able to extend test to other parts and use stub.

If all good I will come with a follow up PR adding the squash commit settings once it this one is merged.

@kairoaraujo
kairoaraujo marked this pull request as ready for review June 5, 2025 14:12
Comment thread otterdog/models/repository.py
@kairoaraujo
kairoaraujo merged commit 0b94084 into eclipse-csi:main Jun 12, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Capture changes for secrets if they are changed from a dummy value

3 participants