-
Notifications
You must be signed in to change notification settings - Fork 121
Decision record for in region checking method #1011
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
Conversation
I will automatically update this comment whenever this PR is modified
|
For those getting notifications, we are working on a process here at Hackday! |
@jhkennedy I had a bit of time this afternoon to get back into this decision. I added some additional context text just to help me better define the problem. Happy to keep iterating on this - I'm also thinking we can combine the "Pros and cons of the options" directly with the "Considered Options" section. Perhaps we can aim to get it out of draft mode by next week? @itcarroll I don't recall how we are expecting the reviewers to vote on their preferred option: Just adding comments here in the PR, is that right? |
Sounds right to me! Is this ready for review? Can we move it out of draft state if so? |
I picked up a bit where @asteiker left off, and I'd say this is ready to start reviewing at this point. Notably, we'd like to use this as a pathfinder for the implementation of the decision committee process that we discussed in the hackday on May 13th. To help with that, I've also opened #1030, which describes at a high level how we discussed the processes could work. Ideally we want it async and a codeowners files, github teams, and PR reviews I think can provide us all the mechanics we need to do this. |
|
||
## Decision Outcome | ||
|
||
Chosen option: "(3) or (4)", because supporting direct access is critical to our community (outright rejecting (1)) and because there is no technical way to do (2) without standing up permanent infrastructure earthaccess isn't funded to maintain. |
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.
I think the main open question is, what do we do by default?
I lean towards (4) because:
- things will just work for general users
- we don't needlessly blast warnings by default
- our more advanced users can specify what they want to happen
but I can see (3) as well so I'm pretty neutral here.
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.
I was leaning a bit more towards (3), because I don't mind warnings — as long as the warnings are worded in a friendly and simply informative way. However, after hearing the arguments for hiding vs teaching and for things to always work for all users, I now prefer option (4). If advanced users know and/or want to use more advanced cloud access patterns, they can seek out and work with those approaches.
606270c
to
0162fda
Compare
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.
I lean towards option 4 due to the "bad" behavior identified in option 3 (generating un-desired warnings by default for likely most of our earthaccess users)
Great write-up! My vote is for (4). I like "always works by default" and opt-in for a "power user" workflow, with good accompanying documentation to educate users on the benefits. |
|
||
## Decision Outcome | ||
|
||
Chosen option: "(3) or (4)", because supporting direct access is critical to our community (outright rejecting (1)) and because there is no technical way to do (2) without standing up permanent infrastructure earthaccess isn't funded to maintain. |
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.
I was leaning a bit more towards (3), because I don't mind warnings — as long as the warnings are worded in a friendly and simply informative way. However, after hearing the arguments for hiding vs teaching and for things to always work for all users, I now prefer option (4). If advanced users know and/or want to use more advanced cloud access patterns, they can seek out and work with those approaches.
67ec575
@mfisher87, @danielfromearth, and @asteiker pushed changes to update to MkDocs style admonitions and specify that we choose option 4, which dismissed your review. Can you check the language in the "Decision Outcome" again and re-approve? For the record, I also agree with option 4. |
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.
Sorry for holding this up! LGTM
Co-authored-by: Matt Fisher <[email protected]>
72e0be7
Since I update the branch and and accepted @mfisher87 suggestion, all the approvals were dismiss. To summarize, this is where things stand:
Since we're at a simple majority of reviewers, with no dissent/issued raised, I think that will satisfy the "endorsement threshold" for our first decision. I'll leave this open until Tuesday morning (Sept. 2) in case anyone else wants to weigh in. Feeback on this decision process would be great -- I've got a draft PR open here: #1030 That's also where we can refine the endorsement threshold and committee members. |
Sorry for falling silent on this one, too, everybody. I also support the decision to work towards option 4. |
Re-approving after the last update. I think we had talked about me merging once a majority consensus was reached, but happy to wait until next Tuesday Sept 2, or of course @jhkennedy feel free to merge as well. I will review #1030 in the meantime. |
Issue that needs: decision : #231
📚 Documentation preview 📚: https://earthaccess--1011.org.readthedocs.build/en/1011/