Skip to content

Update the Google Calendar integration doc. #865

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Niloth-p
Copy link
Contributor

@Niloth-p Niloth-p commented Apr 14, 2025

Follow-up to the Google Calendar integration updates.

Moved from #32783 following the migration of the Python API integration docs.

Performs a complete re-write of the doc, and is quite different from the version in the previous PR.

Screenshots

image
image
image
image
image

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@Niloth-p
Copy link
Contributor Author

@laurynmm Could you please review this?

Copy link

@laurynmm laurynmm left a comment

Choose a reason for hiding this comment

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

@Niloth-p - Thanks for updating this documentation for the updates to this integration! I had some general feedback in an initial read-through. I tried to come at it from the perspective of a person who's not really technical, but who would be interested in this type of integration.

One thing I didn't comment on, but am thinking about is the ordering of where the configuration options information is ... someone implementing this is definitely going to want to review all of that before running the script. Even though it's not the general structure for our docs, I wonder if this should be an instructions section before the one about running the script. I'm not sure though, so maybe focus on the comments below for this pass, and then we can assess how it reads.

Let me know if you have any questions about my review comments below.

@laurynmm laurynmm self-assigned this Apr 18, 2025
@Niloth-p
Copy link
Contributor Author

Wow, that's a pretty thorough review, thanks a lot!
You didn't miss even a hyphen discrepancy :)
Reviewer goals!

I'm formatting my comments and updates in a single block, because I've clubbed multiple questions with single replies, and also so that these comments don't get submerged as the original comments get outdated.

I've absorbed all the phrasing improvements you've suggested. Thank you!

Screenshots

image
image
image
image
image

The macro

So, for Zulip Cloud, this step isn't necessary, correct? Or is this integration only available for self-hosted servers?

Also, just confirming that this can be done on any machine.

Ah, you're right. I just used the macro that was used in the other integrations.
This integration can be run from any system, doesn't have to necessarily be the Zulip server.
Thank you for catching that.
I've removed the macro.
I assume it would be fine to keep the installation step. But, I've moved it to the right section.

Config file example and command arguments example

If the Google Meet link is missing from the payload, the join call link would be broken, correct? I wonder if we want that in the message template if so...

Does this need to be included in the file configuration if it's blank?

Do these need to be included in this example template? I think the options should be listed somewhere, but I imagine most folks aren't going to change these file location defaults.

My intention was to include all the options in the sample config file and the sample command.
I didn't intend for it to be copy pasted, since we have better default values (like the Google meet link case you mentioned), but I can see how users are likely to do that if we provide a full config file. So, I've now made the examples leaner.

And I've moved the format_message sample to the configuration options section, because we might want to show the sample without the users using that exact one. The code logic constructs the message template conditionally, which wouldn't be possible with a fixed template.

Location of configuration options section

someone implementing this is definitely going to want to review all of that before running the script.

I think the usual approach would be to getting it running first, and then configuring it as desired. So, it makes sense to keep the optional configuration options at the end, after the instructions for setting it up. The default values should be robust enough that they wouldn't need a config file or need to pass any parameters, unless they really want to.

Google Cloud instructions

When I followed this link, I had to create a project before I could access that page... Also, I had no idea what Google Cloud was and I imagine a general Google email user wouldn't either.

Oh, you're absolutely right, I've added a new instruction now. But, it's got multiple steps. I didn't want to break it down into individual steps, because they reduce readability, it's helpful to keep the steps for this instruction together?

I've not included a link to the Google Cloud console because it redirects to a very generic welcome page. The specific pages have links though, which are the final URLs for each step.

I've also added the generic link to that in the related docs section.

Calendar ID

Where would one find the ID for a Google calendar?

I'd addressed this here in this comment last year 🫠
"I've removed the instructions on how to get the --calendar value, as they've changed over time and are hence better googled. There's no particular Google documentation to link to either."

I've now made that option the penultimate option, so that it's not the first option that users come across.

For you to test, I'm sharing how I got the calendar ID - Go to Settings, select a calendar (other than "primary" and "birthdays"). In the "Calendar settings" tab for that particular calendar, there's an Integrate calendar section that contains the Calendar ID.

Tokens-file option

When would it make sense to specify this tokens-file option? Does running the script ever update the tokens?

Yes, the script needs to refresh the token every hour. The token expires in an hour. I mentioned "first run" because the file doesn't exist until then. Inspired by your question, I've now added that it gets rewritten every hour.

"{No title}", "{No description}", "{No location}", and "{No link}"
in the message template.

The default message template takes the following form when all the event
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 plan to update this line to "The example screenshot below uses the default message template, which takes the following form when all the event field variables are non-empty." once we have the screenshot setup done, which ideally shouldn't take too long. But, we still wouldn't want to mention a missing screenshot, so I've phrased it this way for the moment.

@laurynmm
Copy link

@Niloth-p - Thanks for making those updates and that comprehensive summary note! I think this is much easier to read and understand now.

A few notes I have after reading through the screenshot of the doc above:

  1. I think we can remove the "Configure the integration" header and just put all of that under "Configuration options".
### Configuration options

The integration can be configured by:
- Passing command line arguments to the integration script.
- Editing the `google-calendar` section of the `zuliprc` file.

The configuration settings in `zuliprc` will be overridden by the
corresponding command line options, if both are used.

The integration script accepts the following configuration options:
- `interval`: How many minutes in advance you want reminders delivered.
  The default value is 30 minutes.
  1. Question about steps 3 and 4 in the "Run the integration script" section. Does it matter if you run the script without the command line arguments and then run it again with them? I assume that one could run the script with various arguments to test how one wants to eventually configure the zuliprc file? Maybe step 4 should say "reconfigure" vs "configure". Or it could be formatted as a bullet point under step 3. I'm not sure what would be the best/clearest.

  2. I think that instances of "commandline" should be "command line", correct?

  3. I assume if the terminal session is closed/ended, then the user only needs to rerun the script in a new session to get the integration running again. If so, it might be worth noting that in step 5 of running the integration script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants