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

Update Makefile from starter template, upgrade node to v16, and remove mattermost-webapp dependency #229

Merged

Conversation

mickmister
Copy link
Contributor

Summary

Ticket Link

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3583322) 6.42% compared to head (727010e) 6.42%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #229   +/-   ##
======================================
  Coverage    6.42%   6.42%           
======================================
  Files          11      11           
  Lines        1712    1712           
======================================
  Hits          110     110           
  Misses       1594    1594           
  Partials        8       8           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor Author

@mickmister mickmister Nov 14, 2023

Choose a reason for hiding this comment

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

I removed this file since the component this is testing hasn't changed since introduced, and the test hasn't provided much value. The "requirement" of removing had to do with removing enzyme, which required some dependencies that we don't want to keep around going forward. For plugins that have meaningful React tests, we'll want to replace usage of enzyme with @testing-library/react, just as the main webapp project has been using.

@mickmister mickmister added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 14, 2023
@mickmister
Copy link
Contributor Author

@AayushChaudhary0001 This should just require a general smoke test

@mickmister mickmister mentioned this pull request Nov 14, 2023
@mickmister mickmister changed the title Upgrade node to v16 and remove mattermost-webapp dependency Update with starter template, upgrade node to v16, and remove mattermost-webapp dependency Nov 14, 2023
@@ -0,0 +1,33 @@
module github.com/mattermost/mattermost-plugin-starter-template/build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added build/go.mod to lessen the scope of this PR a bit. This way we don't introduce a bunch of changes to use mattermost/server/public in this PR. That will be done in a separate effort to sync all plugin projects with starter template

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call 👍

@mickmister mickmister changed the title Update with starter template, upgrade node to v16, and remove mattermost-webapp dependency Update Makefile from starter template, upgrade node to v16, and remove mattermost-webapp dependency Nov 14, 2023
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Nice work 👍

@@ -0,0 +1,33 @@
module github.com/mattermost/mattermost-plugin-starter-template/build
Copy link
Contributor

Choose a reason for hiding this comment

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

Good call 👍

@hanzei hanzei removed the 2: Dev Review Requires review by a core committer label Nov 16, 2023
@mickmister
Copy link
Contributor Author

Smoke test passes

@mickmister mickmister merged commit fd47bbe into mattermost-community:master Nov 16, 2023
4 checks passed
@avas27JTG avas27JTG added this to the v1.0.0 milestone Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants