Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

[Decision] Commit function #4777

Closed
wants to merge 5 commits into from

Conversation

flo91
Copy link
Collaborator

@flo91 flo91 commented Dec 14, 2022

This PR is for working on the decision about the commit function (communicating the current phase to plugins).

It should at least reach the state "solutions clear" before closing this PR.

Basics

  • Short descriptions of your changes are in the release notes
    (added as entry in doc/news/_preparation_next_release.md which
    contains _(my name)_)
    Please always add something to the release notes.
  • Details of what you changed are in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, are in the commit messages.
  • The buildservers are happy. If not, fix in this order:
    • add a line in doc/news/_preparation_next_release.md
    • reformat the code with scripts/dev/reformat-all
    • make all unit tests pass
    • fix all memleaks
  • The PR is rebased with current master.

Checklist

  • I added unit tests for my code
  • I fully described what my PR does in the documentation
    (not in the PR description)
  • I fixed all affected documentation (see Documentation Guidelines)
  • I fixed all affected decisions (see Decision Process)
  • I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
  • I updated all meta data (e.g. README.md of plugins and METADATA.ini)
  • I mentioned every code not directly written by me in reuse syntax

Review

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the basics are fulfilled and no further pushes are planned by you.

Copy link
Member

@kodebach kodebach left a comment

Choose a reason for hiding this comment

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

Please update the existing decision to match the new template. Specifically, please switch to using subsections in the "Considered Alternatives". That makes is a lot easier to read, write and talk about options.

IMO the decision can be moved to at least "In Discussion", but probably even "In Progress".

Copy link
Contributor

@eiskasten eiskasten left a comment

Choose a reason for hiding this comment

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

Well done! May you give some examples for function which potentially get called multiple times? I.e. if they are contract functions, plugin internal functions, etc.

@flo91 flo91 force-pushed the decision-commitFunction branch 2 times, most recently from fa64ee7 to 5254e2b Compare July 12, 2023 01:51
to the decision `doc/decisions/0_drafts/commit_function.md`
- move to state "solutions clear"
- add new assumption
- extend the description of some possible solutions
@flo91 flo91 marked this pull request as ready for review July 12, 2023 03:48
@flo91 flo91 added needs review please review this PR and removed work in progress documentation labels Jul 12, 2023
@flo91 flo91 requested a review from markus2330 July 12, 2023 03:49
flo91 and others added 2 commits July 12, 2023 06:01
- describe the current situation in more detail
- mention the affected plugin functions
Copy link

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new PR with the remainder of this PR.
Thank you for your contributions 💖

@github-actions github-actions bot added the stale label Jul 14, 2024
Copy link

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new PR with the remainder of this PR.
Thank you for your contributions 💖

@github-actions github-actions bot closed this Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants