Skip to content
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

Plugin review proposals #3

Merged
merged 6 commits into from
Jan 30, 2020
Merged

Plugin review proposals #3

merged 6 commits into from
Jan 30, 2020

Conversation

jfrerich
Copy link
Contributor

Summary

This is a PR with suggested changes discovered during the Plugin Review Process. The purpose of this 2nd PR is to give both the plugin author and other developers the chance to review changes before pushing into master.

@jfrerich jfrerich requested review from marianunez and levb January 29, 2020 01:03
@jfrerich
Copy link
Contributor Author

Hi @marianunez and @levb. This is a PR that can be used to let the authors and devs see the proposed changes I would make. Note this is currently a draft PR and I will be making for commits.

This was referenced Jan 29, 2020
@levb levb added the 2: Dev Review Requires review by a core committer label Jan 30, 2020
@levb
Copy link

levb commented Jan 30, 2020

1/5 for merging incrementally

@marianunez
Copy link
Contributor

1/5 for merging incrementally

@levb Can you explain what you mean by incrementally?

@levb
Copy link

levb commented Jan 30, 2020

@marianunez Like, in merge the changes we already have here (once approved), and cut another PR if we need to make more changes.

@marianunez
Copy link
Contributor

@levb sounds good. I discussed offline with @jfrerich and I think most of the other changes proposed I will continue on the original PR here #2

@marianunez marianunez marked this pull request as ready for review January 30, 2020 16:24
@jfrerich
Copy link
Contributor Author

@levb, for this plugin, I have created this PR so that @marianunez can review the changes before merging. This is a little different than the google calendar plugin, where I basically took ownership and merged changes directly.

@marianunez, if these changes look good for your approval, let's go ahead and approve/merge and I'll create additional PRs for other changes.

@jfrerich jfrerich added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Jan 30, 2020
@jfrerich jfrerich merged commit 47f2c5b into pluginReview Jan 30, 2020
@jfrerich jfrerich deleted the plugin-review-proposals branch January 30, 2020 17:13
marianunez added a commit that referenced this pull request Mar 19, 2020
* Full commit without template for review

* Linting

* Plugin review proposals (#3)

* spelling

* spelling

* fix make golint errors

* Break up sentence into multiple

* Spelling fixes in webapp and server
err var renamed to appErr when model.AppError is return type

* remove broken to ticket.  The fix is now in mm-webapp master

* go mod tidy

* Updated Readme. Dropped plugin in plugin id

* Added Sat-Sun. Dropped plugin in name

* Weekdays fixes

* PR Feedback

* Fixing Makefile

* Fixing merge conflicts

* Added help command

* Updated UI

* linting

* More linting. Sorry for the churn here

* Update go module name

* checkin results from npm-check -E -y

* remove references to starter template
remove public/hellow.html

* fix lint error

* Add Contributing section to README

Co-authored-by: Jason Frerich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants