-
Notifications
You must be signed in to change notification settings - Fork 103
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
docs: add ecosystem service credit RFC + RFC template #107
Conversation
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
===========================================
- Coverage 76.90% 61.03% -15.87%
===========================================
Files 101 91 -10
Lines 12522 9021 -3501
===========================================
- Hits 9630 5506 -4124
- Misses 2288 3042 +754
+ Partials 604 473 -131
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a README.md that indexes the RFC's? Maybe include a link in the README &/or template to the RFC process we're trying to follow.
Great overview and intro. Also, is there any difference between this document and the RFC: Ecosystem Service Credit Module Google Doc? Would be good to keep things in one place, otherwise one will get outdated. So... maybe we should point from one document to another, and in the slave one remove the common content (maybe except the abstract) and point to the master document. |
To be honest, I find it easier to read the spec docs in a more "Higher level" doc management system, like:
All solutions above have a nice commenting system and also allow to trace edits with notifications / emails. The advantage of each of that systems is that we observe a single working document rather than bunch of issues and pull-requests. |
Could you add a README @clevinson and then we'll merge as is and iterate if needed? |
I think there were few updates in the Google Doc, could be good to port them and freeze the google doc. |
Ping @clevinson |
Is this PR actually R4R? What should we do about it? |
Updated w/ @aaronc feedback adding a README & pushing this R4R again! I'd love to get this merged finally :) Happy to iterate on this in the future, but my preference is that we start with this process (maybe initially with the ecocredit updates that we've already discussed implementing?), and then look to iterating once we have some momentum on creating these documents again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. A few minor suggestions but nothing blocking.
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Co-authored-by: Ryan Christoffersen <[email protected]>
Just noticed a typo in the filename: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few nits, pre-approving
|
||
## Complex State Machines | ||
|
||
Another design decision made in this proposal was to only implement two possible states for credits: “tradable” and “retired”. These are the two known states that we need to be able to manage for our first use case with the registry. While it is assumed that there may be more complex state machines that we will need to support at a later date, we chose to start with satisfying the current use case for the same reasons illustrated above (reducing complexity, and allowing future use cases to drive more complex functionality or generalizations). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another design decision made in this proposal was to only implement two possible states for credits: “tradable” and “retired”. These are the two known states that we need to be able to manage for our first use case with the registry. While it is assumed that there may be more complex state machines that we will need to support at a later date, we chose to start with satisfying the current use case for the same reasons illustrated above (reducing complexity, and allowing future use cases to drive more complex functionality or generalizations). | |
Another design decision made in this proposal was to only implement two possible states for credits: “tradable” and “retired”. These are the two known states that we need to be able to manage for our first use case with the registry. While it is assumed there may be more complex states that we will need to support at a later date, we chose to start with satisfying the current use case for the same reasons illustrated above (reducing complexity, and allowing future use cases to drive more complex functionality or generalizations). |
i think you meant only states
here instead of state machines
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - meant state machines (Right now the state machine is tradable
-> retired
, as the only allowable state transition possible). It may be in the future that we have other state transitions possible (to newly defined states).
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Co-authored-by: Tyler <[email protected]>
Thanks @ryanchristo @technicallyty ! |
ref #78
Proposed Changes
Feel free to submit a Draft Pull Request as soon as you start working on something. Before you mark your PR as Ready For Review, make sure you've checked off the following boxes or indicated N/A (not applicable):
CHANGELOG.md
following https://keepachangelog.comThanks!