Added the mattermost-webapp from the monorepo of mattermost #1074
Conversation
There was a problem hiding this comment.
These seem like unrelated changes
There was a problem hiding this comment.
I was facing a very peculiar error https://github.com/mattermost/mattermost-plugin-jira/actions/runs/8938919453/job/24553976441, so there was a package named file-hound of frontend, and after it was installed, it was creating an empty go file, and in the lint as well as test CI, we run commands like go test ./... which basically going to work on all files including the node modules and that's why we were getting the above error. So for now I have made the go commands to only work in the server folder and not outside of it.
@hanzei
There was a problem hiding this comment.
I think the problem @hanzei may have with this is that the build folder is no longer being checked. Can we instead make it so only webapp exists in the monorepo checkout folder? Either we can do something replacing the whole repo with just the webapp directory like:
mv mattermost-webapp/webapp .
rm -rf mattermost-webapp
mv webapp mattermost-webappNaming the cloned directory just mattermost may make more sense here. Then this becomes:
mv mattermost/webapp .
rm -rf mattermost
mv webapp mattermost-webappAlternatively we can do a sparse checkout, but that seems more complicated https://stackoverflow.com/a/52270636
There was a problem hiding this comment.
Nice work on this @Kshitij-Katiyar 🚀
In general LGTM 👍 Just some comments for discussion, and one request to move the monorepo commit hash to package.json to keep all the dependency information in one place.
webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx
Outdated
Show resolved
Hide resolved
| @@ -69,8 +69,8 @@ endif | |||
| # weird reports at golangci-lint step | |||
| ifneq ($(HAS_SERVER),) | |||
| @echo Running golangci-lint | |||
| $(GO) vet ./server/... | |||
| $(GOBIN)/golangci-lint run ./server/... | |||
| $(GO) vet ./server/... ./build/... | |||
There was a problem hiding this comment.
Do we still need this? Or will $(GO) vet ./... work since we changed how the monorepo checkout works?
There was a problem hiding this comment.
The reason I ask, is we want to keep the Makefile as identical to the starter template's Makefile as we can. This change creates drift between the two repos
There was a problem hiding this comment.
@mickmister Yeah we still require the file-hound package and its respective files.
Summary
Added the mattermost-webapp from the mono repo of mattermost to add the newer mattermost-redux package. Took the reference from the https://github.com/mattermost/mattermost-plugin-calls/pull/555/files.