-
Notifications
You must be signed in to change notification settings - Fork 183
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
Introducing Metrics in InnerSource: Merge PR497 and PR501 upstream (instead of new patterns) #526
Conversation
All forces mentioned were against the pattern. I'm adding some in favor for balance and better justification.
Retrieve known instance from InnerSourceCommons#497
Structure the solution. Mention outliers (than you, Addie).
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.
@fioddor thanks for consolidating the different PRs related to this pattern. You approach sounds good to me.
I left some comments inline. Will merge the straightforward fixes myself.
Aside:
As you are adding a Known Instance to this pattern, we could upgrade this pattern to Structured soon. I suggest to do that via a separate PR though, once all the others are merged or closed. That allows us to do one more proper review and check the pattern against all requirements of the Structured level.
Links to metrics references that are mentioned in the text. Co-authored-by: Sebastian Spier <[email protected]>
Minor style fixes. Co-authored-by: Sebastian Spier <[email protected]>
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.
LGTM
PR497 duplicates this pattern.
PR501 might extend this pattern instead of creating a new one.
This MR intends to capture the useful bits and merge them upstream so PR497 and/or PR501 can be withdrawn.