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.

Sign up for GitHub

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 Repository for kuksa-java-sdk #33

Merged
merged 2 commits into from
Dec 12, 2024
Merged

Add Repository for kuksa-java-sdk #33

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28aa56b
Add Repository for kuksa-java-sdk
wba2hi Dec 11, 2024
2d2c94c
Add Description
wba2hi Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions otterdog/eclipse-kuksa.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,5 +416,19 @@ orgs.newOrg('eclipse-kuksa') {
kuksa_default_branch_protection_rule('main')
],
},
orgs.newRepo('kuksa-java-sdk') {
wba2hi marked this conversation as resolved.
Show resolved Hide resolved
allow_merge_commit: true,
allow_rebase_merge: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we use the same merge strategy as for the android-repos, i.e. only allow merge-commits, do not allow rebase/squash. As I understood there were some reasons to have that setup for Android, but does those reasons apply here as well, or do we want to have same setup as for most other repos where we allow all three methods?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind changing the approach here. I would like some kind of consistency though. The approach here is based on GitLab Flow. Are the other repos based on some official branching flow?

However there are some disadvantages of squashing with the current commit approach (conventional commits). The commits are linted for a consistency. Automatic Squash Merges will reduce the usefulness of this. So I am against that one. However you can squash it manually and force push before the merge if you want.

We can switch from merge commits to always rebase here if that feels better.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would tend to rebase merge as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not have any official branching flow in other repos. Typically there seems to be two sorts of people, those who prefer a clean history and fast forward merge, and those who prefer merge-commits. So in the end regardless of policy we will annoy some people. Personally I prefer a clean history unless the source branch is something long-lived, like merging "main" to "production" if you use the GitLab flow

There are also different views on how to keeping feature branches up-to-date, some prefer rebase and force-push to keep linear history, other prefer merges to make sure that there are no "orphan" comments. The general risk I see if you use merge-commits (or rebase) is that contributors will not do a rebase/squash/force-push to the feature branch before merge, so you end up with a lot of in my view use-less commit messages like "Applying suggestions from code-review"

The problem with too aggressive commit linters is that you might get errors also for commits created by bots like in eclipse-kuksa/kuksa-android-sdk#119, see https://github.com/eclipse-kuksa/kuksa-android-sdk/actions/runs/12070583813, as dependabot does not follow "subject must be sentence-case"

Copy link
Contributor

@Chrylo Chrylo Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always a hot topic because the pros and contras are difficult to clearly compare.^^

I suggest:

  • Doing rebase for the clean history so it is aligned with most other repos
  • Keep using conventional commit and automatic changelog / release note generation. The advantage is too big IMHO.
  • Do not allow merge commits for consistency. Either rebase or merge commits.
  • Do not allow automatic squashing because it should be done deliberately to ensure a good commit quality for the conventional commits standard.
  • Document a clear Branching / Release concept (Preferable taking an official one) so all the rules are clearly linked and can be easily copied to a new repo.

If we don't care about the commit history or automatic changelog generation then we can think about allowing "everything". Could be that the advantages are not worth the trade off. But that is currently the basis for my arguments. :)

The risk that OSS Contributors are restricted too much by linters etc. to contribute is always there with every quality gate we are introducing. Be it code quality or commit quality tools. My feeling is that a good commit quality is ignored too much. That is why the main contributor Role is so important. You probably never want to just merge a (bigger) PR from outside without cleaning it up.

We should probably also not enforce "subject must be sentence-case" since dependabot is already using the conventional commit standards OR change the config for the dependabot for the commit message (if possible).

@erikbosch I let you decide together with Andre as the main reviewer one here without hard feelings. My motivation is only to solve the "bad commit messages" issue and automate release notes generation.

allow_squash_merge: false,
allow_update_branch: false,
dependabot_security_updates_enabled: true,
web_commit_signoff_required: false,
workflows+: {
actions_can_approve_pull_request_reviews: false,
},
branch_protection_rules: [
kuksa_default_branch_protection_rule('main')
],
},
],
}