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

chore(satp-hermes): add maintainers to satp-hermes package #3734

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

RafaelAPB
Copy link
Contributor

@RafaelAPB RafaelAPB commented Jan 30, 2025

chore(satp-hermes): add maintainers to satp-hermes package

This PR adds @LordKubaya and @AndreAugusto11 as maintainers for the satp-hermes package.

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

[skip ci]

@RafaelAPB
Copy link
Contributor Author

@petermetz @ryjones can you provide write access to the satp-dev branch to @AndreAugusto11 ? I'm getting an error from the CODEOWNERS file Unknown owner on line 18: make sure @AndreAugusto11 exists and has write access to the repository

@petermetz
Copy link
Contributor

@RafaelAPB I think we need to PR in the governance repo to add him into the team first. I'll send the PR after the TAC meeting (which is starting right now)

@RafaelAPB RafaelAPB force-pushed the satp-maintainers-update branch from c9efc12 to 0a9cabe Compare January 30, 2025 15:05
@ryjones
Copy link
Contributor

ryjones commented Jan 30, 2025

@RafaelAPB I think we need to PR in the governance repo to add him into the team first. I'll send the PR after the TAC meeting (which is starting right now)

correct

@RafaelAPB
Copy link
Contributor Author

RafaelAPB commented Jan 30, 2025

Cross-posting for history and visibility @petermetz : hyperledger-cacti/governance#9

@petermetz
Copy link
Contributor

Cross-posting for history and visibility @petermetz : hyperledger-cacti/governance#9

Nice, thank you @RafaelAPB
Now that that's merged, this should stop having syntax errors as well.

@ryjones
Copy link
Contributor

ryjones commented Jan 30, 2025

@petermetz I don't know how the commit message parity works.

@petermetz
Copy link
Contributor

@petermetz I don't know how the commit message parity works.

For now, it doesn't. Safe to ignore until we stabilize it (but having it run is the only way to discover all the edge cases).
The intent behind is to avoid (the very common) situation where people type a full page of documentation into the PR description but then the commit message itself is "updated some files" and that's it.
In the unlikely but non-zero chance event of us ever moving to a different git host I want to make sure that we can carry with us as much context in the git messages as possible instead of it being stuck in a closed database of someone else (the PR descriptions)

@ryjones
Copy link
Contributor

ryjones commented Jan 30, 2025

OK. I'll leave it alone.

@petermetz
Copy link
Contributor

OK. I'll leave it alone.

Cheers and sorry for the confusion about it. I wish it was working fine already, but there are a lot of edge cases to it.

@RafaelAPB RafaelAPB force-pushed the satp-maintainers-update branch from 0a9cabe to 8b30ffb Compare January 31, 2025 08:08
@RafaelAPB
Copy link
Contributor Author

@petermetz should be good to go

@petermetz
Copy link
Contributor

@petermetz should be good to go

@RafaelAPB We just need one more reviewer and then the merge button will get enabled.

@hyperledger-cacti/cacti-maintainers please review when you get the chance!

@jagpreetsinghsasan
Copy link
Contributor

@petermetz I don't know how the commit message parity works.

For now, it doesn't. Safe to ignore until we stabilize it (but having it run is the only way to discover all the edge cases). The intent behind is to avoid (the very common) situation where people type a full page of documentation into the PR description but then the commit message itself is "updated some files" and that's it. In the unlikely but non-zero chance event of us ever moving to a different git host I want to make sure that we can carry with us as much context in the git messages as possible instead of it being stuck in a closed database of someone else (the PR descriptions)

True that @ryjones @petermetz
I think if we dont specify the line stating This PR adds ... as maintainers... (or mention the same in commit message) it will fix the commit parity. Basically the idea being the PR description should closely match to that of the commit message. And the above mentioned line is straightforward from the Files changed section of the PR.
We can straightaway ignore the test for certain PRs (like this), or if you want can I try re-checking after changing the PR description @petermetz ?

@outSH
Copy link
Contributor

outSH commented Feb 14, 2025

Sorry for late review, LGTM of course

@ryjones ryjones force-pushed the satp-maintainers-update branch from 8b30ffb to c38902d Compare February 14, 2025 14:46
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>
@petermetz petermetz force-pushed the satp-maintainers-update branch from c38902d to 41c135d Compare February 16, 2025 22:34
@ryjones ryjones merged commit ca70682 into hyperledger-cacti:main Feb 17, 2025
136 of 141 checks passed
@petermetz petermetz deleted the satp-maintainers-update branch February 18, 2025 17:38
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.

5 participants