FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Review crossteam-project-valuation by spier · Pull Request #261 · InnerSourceCommons/InnerSourcePatterns · GitHub

/ InnerSourcePatterns Public

Review crossteam-project-valuation - #261

Merged
spier merged 6 commits into
masterfrom
review-crossteam-project-valuation
Jan 22, 2021
ConversationCommits (6)ChecksFiles changed
Merged

Review crossteam-project-valuation#261
spier merged 6 commits into
masterfrom
review-crossteam-project-valuation

Conversation

spier commented Dec 27, 2020

Copy link
Copy Markdown
Member

Part of #259.

The changes so far are mostly:

  • Formatting fixes in the markdown.
  • Changes to spelling of InnerSource.

I have some further questions for @rrrutledge that I will put into inline comments.

spier requested a review from rrrutledge December 27, 2020 16:07
spier commented Dec 27, 2020
View reviewed changes
Comment thread patterns/2-structured/crossteam-project-valuation.md Outdated
spier commented Dec 27, 2020
View reviewed changes
Comment thread patterns/2-structured/crossteam-project-valuation.md Outdated
spier mentioned this pull request Dec 27, 2020
Closed
lenucksi added the 📖 Type - Content Work Working on contents is the main focus of this issue / PR label Jan 1, 2021
lenucksi approved these changes Jan 1, 2021
View reviewed changes

lenucksi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Interesting changes. Let's get the discussions resolved and merge it.

Comment thread patterns/2-structured/crossteam-project-valuation.md Outdated
spier mentioned this pull request Jan 4, 2021
Open
spier and others added 2 commits January 17, 2021 23:07

spier commented Jan 17, 2021

Copy link
Copy Markdown
Member Author

(moved inline comment)

This pattern contains 3 extra sections that are not part of the regular pattern template format:

  • Explanation
  • Formula
  • Commentary

Some thoughts:

  • I wonder if these should be considered sub-sections to "Solution"? (in which case we could move them one heading level down i.e. to ###. This would likely be the quickest fix to make the format of this pattern more consistent with other.s
  • Alternatively we could work the content from the 3 extra sections into other sections?
  • Explanation could maybe be renamed to Rationale?

spier commented Jan 17, 2021

Copy link
Copy Markdown
Member Author

I went for the least invasive chance which was to move the 3 sections Explanation/Formula/Commentary as sub-sections under 'Solution'.

@rrrutledge if you could review these changes to check that we didn't misrepresent anything here, that would be awesome.

As these are mostly formatting changes, typo fixes, and minimal changes to make the pattern fit he pattern template, I would otherwise merge this PR towards the end of next week or so.

spier commented Jan 22, 2021

Copy link
Copy Markdown
Member Author

Reaching self-set timer of this PR :). Merging.

spier merged commit 2853336 into master Jan 22, 2021
spier deleted the review-crossteam-project-valuation branch January 22, 2021 04:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lenucksi lenucksi approved these changes
rrrutledge Awaiting requested review from rrrutledge
cewilliams Awaiting requested review from cewilliams
gruetter Awaiting requested review from gruetter
NewMexicoKid Awaiting requested review from NewMexicoKid
nyeates Awaiting requested review from nyeates

Assignees

No one assigned

Labels

📖 Type - Content Work Working on contents is the main focus of this issue / PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL