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

v1.0.0 Google Calendar Plugin #41

Merged
merged 10 commits into from
Oct 5, 2023
Merged

v1.0.0 Google Calendar Plugin #41

merged 10 commits into from
Oct 5, 2023

Conversation

fmartingr
Copy link
Contributor

@fmartingr fmartingr commented Oct 4, 2023

Summary

This pull request introduces the v1.0.0 Google Calendar release created in the https://github.com/mattermost/mattermost-plugin-mscalendar repository as groundwork for sharing common code for similar plugins.

This PR leverages the work done in that plugin to allow some of the features and allow compatibility with HA environments, check the README.md for more details on features.

@fmartingr fmartingr self-assigned this Oct 4, 2023
@fmartingr fmartingr requested review from mickmister and hanzei October 4, 2023 09:09
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.

50k lines of code is a bit too much to review in one PR. What are the files that I specifically should take a look at?

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Oct 4, 2023
@fmartingr
Copy link
Contributor Author

50k lines of code is a bit too much to review in one PR. What are the files that I specifically should take a look at?

Code is mostly the same we had in the mscalendar plugin, so the main server, and that general things for the plugin are in place and I haven't forgot to migrate anything.

Not asking for an in-depth review of course, just that what I migrated makes sense, logic was reviewed some time ago.

@codecov-commenter
Copy link

Codecov Report

All modified lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@ac997a6). Click here to learn what that means.

Additional details and impacted files
@@            Coverage Diff            @@
##             master      #41   +/-   ##
=========================================
  Coverage          ?   11.22%           
=========================================
  Files             ?       16           
  Lines             ?      606           
  Branches          ?        0           
=========================================
  Hits              ?       68           
  Misses            ?      536           
  Partials          ?        2           

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

@mickmister
Copy link
Contributor

Reusing the same repo as the archived one seems weird, but I saw the discussion on it and I suppose that's the plan we're going with. What was your method of getting the code onto this repo?

@fmartingr
Copy link
Contributor Author

Reusing the same repo as the archived one seems weird, but I saw the discussion on it and I suppose that's the plan we're going with. What was your method of getting the code onto this repo?

I'd rather go with this without breaking release semver (which we are not) than having a lot of repositores for the same thing. And we are going to have the history anyway.

Essentially, I removed all the files and replaced them with the ones from the Google Calendar plugin PoC that I did the other day.

@fmartingr fmartingr added the Docs/Needed Requires documentation label Oct 5, 2023
@fmartingr
Copy link
Contributor Author

Adding the Docs/Needed label since the repository URL has changed, and to get some eyes on the README.md.

@fmartingr fmartingr merged commit 3da9757 into master Oct 5, 2023
4 checks passed
@fmartingr fmartingr deleted the next-v1.0.0 branch October 5, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer Docs/Needed Requires documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants