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 CODEOWNERS #4544

Merged
merged 7 commits into from
May 15, 2024
Merged

Add CODEOWNERS #4544

merged 7 commits into from
May 15, 2024

Conversation

wileyj
Copy link
Contributor

@wileyj wileyj commented Mar 15, 2024

@jcnelson @kantai @obycode @jferrant @xoloki

could use some help here to iron out any additional paths that should be added .
Once enforced, the idea is to require a codeowner (listed in the file in this PR) to approve a PR before it can merge.
by default, jcnelson/kantai/obycode will be added to all PR's unless specified later in the file (ex: wileyj/charliec3 will be added to PR's changing CI workflows).

what i've added so far is a best guess based on commit history, but i think it can/should be ironed out a bit more before enforcing.

https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

# These owners will be the default owners for everything in
# the repo. Unless a later match takes precedence,
# @global-owner1 and @global-owner2 will be requested for
# review when someone opens a pull request.
* @jcnelson @kantai @obycode

# Generic file extensions that shouldn't require much scrutiny
*.md @stacks-network/blockchain-team
*.yml @stacks-network/blockchain-team
*.yaml @stacks-network/blockchain-team
*.txt @stacks-network/blockchain-team
*.toml @stacks-network/blockchain-team

# Signer code
libsigner/**/*.rs @jferrant @xoloki @jcnelson @kantai
stacks-signer/**/*.rs @jferrant @xoloki @jcnelson @kantai

## CI workflows
./github/workflows/ @wileyj @CharlieC3
./github/actions/ @wileyj @CharlieC3

@wileyj wileyj added enhancement Iterations on existing features or infrastructure. security Problem that potentially risks product, data, or other security. labels Mar 15, 2024
@wileyj
Copy link
Contributor Author

wileyj commented Mar 15, 2024

for now, keeping user id's is fine - but i'm also thinking that some new teams in the org would be be easier to manage longer-term.
once we have a good list of who should be approving PR's for specific parts of the codebase, i'll adjust to add teams so any changes won't require another PR.

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.46%. Comparing base (3adb6ed) to head (c6b668f).
Report is 317 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #4544      +/-   ##
==========================================
+ Coverage   83.35%   83.46%   +0.11%     
==========================================
  Files         453      471      +18     
  Lines      328034   337957    +9923     
  Branches      323      317       -6     
==========================================
+ Hits       273431   282076    +8645     
- Misses      54595    55873    +1278     
  Partials        8        8              

see 131 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3adb6ed...c6b668f. Read the comment docs.

@jferrant
Copy link
Collaborator

I have no problem with it :)

jcnelson
jcnelson previously approved these changes Mar 18, 2024
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

Works for me! Happy to delegate if needed

@wileyj
Copy link
Contributor Author

wileyj commented Mar 18, 2024

thanks! once i setup appropriate teams i'll adjust the file.
in the meantime, if there specific changes anyone would like to see let me know and i'll add

obycode
obycode previously approved these changes Mar 18, 2024
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

👍

xoloki
xoloki previously approved these changes Mar 18, 2024
Copy link
Collaborator

@xoloki xoloki left a comment

Choose a reason for hiding this comment

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

LGTM

@wileyj wileyj dismissed stale reviews from xoloki, obycode, and jcnelson via c6b668f March 22, 2024 19:36
jferrant
jferrant previously approved these changes Mar 28, 2024
* next: (282 commits)
  Bugfix: verify block transactions only when the upcoming signers approved key is not set
  Fix bad merge 2
  Fix bad merge
  CRC: change debug to info for pre epoch 3.0 cycles requiring a fee
  Add unit tests for is_in_prepare_phase and is_in_reward_cycle functions
  CRC: remove duplicate logging
  Reward cycle length was being incorrectly stored as reward phase length
  Consider nonce when checking for existing vote transactions in stackerdb during update_dkg
  Fix bad merge
  feat: more efficient tuple types, reduce type map tracking overhead
  CRC: do not pop the command until its executed
  CRC: add config tests and remove config log
  CRC: remove dead path and update debug message to a warn
  Update stacks-signer to attempt to estimate the transaction fee if a user sets max_tx_fee_ustx in the config
  Seperate unsigned and signed transaction building and update tests
  Add get_medium_estimated_fee_ustx to get estimated fee from stacks node
  Remove unnecessary retries throughout code
  fix: don't log aggregate public keys as missing so often (do so at the caller's option), and in the p2p stack in particular, remember that aggregate public keys are never going to appear if we're pre-3.0
  feat: dont pretty-print json output
  chore: Update "stacks-blockchain" -> "stacks-core"
  ...
* next:
  update per 4638
  removing portable flag
  fix: dont warn about aggregate key in 2.5
@wileyj wileyj marked this pull request as ready for review May 9, 2024 16:27
@wileyj wileyj requested review from a team May 9, 2024 16:28
@wileyj
Copy link
Contributor Author

wileyj commented May 9, 2024

now that 2.5 is out, should this be rebased against develop?

@wileyj wileyj marked this pull request as draft May 9, 2024 20:25
* develop: (420 commits)
  remove extra character from vi typo
  remove paths-ignore
  4746 - avoid extra ci runs
  chore: fix comment typo
  fix: more PR feedback -- better, more-complete AtcRational tests and better abstraction over the inner u256
  chore: address PR feedback and add more test coverage
  chore: add unit test
  chore: add unit test
  feat: add no-op metrics timer when feature disabled
  chore: get all unit tests to pass
  chore: API sync
  chore: add a mode of generating burnchain blocks in TestPeer that ensures that a block's hash will depend on the (number of) opts it contains
  fix: don't start from reward cycle 0 when doing an inv sync; start from connection_opts.inv_reward_cycles
  chore: expose set_initial_peer
  fix: optionally disable resolving bootstrap nodes, so the caller can avoid needless network I/O if it's not needed
  chore: use the burnchain indexer to find the right block header
  fix: revert changes in contrib/core-contract-tests/
  fix: typos
  chore: index block-commits by apparent sender, so we can get the last commit by sender
  fix: generate the expected block commit input so that mining works in nakamoto
  ...
@wileyj wileyj changed the base branch from next to develop May 9, 2024 20:31
@wileyj wileyj marked this pull request as ready for review May 9, 2024 20:32
@wileyj
Copy link
Contributor Author

wileyj commented May 9, 2024

rebased against develop - diverging from the PR text a little, i've moved to a team based approach vs hard-coding usernames in the file. this will make it easier to maintain.

there are some paths in the repo that i wasn't sure should have a team directly added as a codeowner, but i'm open to adjusting based on any suggestions.

@wileyj wileyj enabled auto-merge May 14, 2024 16:10
@wileyj wileyj added this pull request to the merge queue May 15, 2024
Merged via the queue into develop with commit f7d8f7e May 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Iterations on existing features or infrastructure. security Problem that potentially risks product, data, or other security.
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants