Skip to content

Conversation

@Niloth-p
Copy link
Contributor

We want to use only ISO 8601 formats with the <time:> syntax, so we remove the previous comment recommending the usage of <time:dep_time>, and instead add a (commented-out) function that converts the datetime to the right format.

How did you test this PR?
I did not.

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.

While the message template customization function had `dep_time` and
`dep_id` parameters, the `deployment` dict was not populated with those
fields previously.
# To utilize Zulip's global times (https://zulip.com/help/global-times),
# uncomment the below function, and call it from format_deployment_message.
# def get_global_time(dt_str: str) -> str:
# return f"<time:{datetime.strptime(dt_str, '%Y-%m-%d %H:%M:%S %z').isoformat()}>"
Copy link
Member

Choose a reason for hiding this comment

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

This won't work without datetime being imported. I think it's fine to have this function available and just unused by default with a comment.

II suggest you also can also put a copy of the datetime=>HTML helper we added in zerver.lib into the main zulip Python library so it can be used by integrations easily.

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