gr2m / merge-schedule-action

GitHub Action to merge pull requests on a scheduled day
ISC License
140 stars 34 forks source link

Comments added by Merge Schedule always specifies UTC time #94

Open atoftegaard-git opened 11 months ago

atoftegaard-git commented 11 months ago

Comments added by Merge Schedule always specifies UTC time, even though another time zone has been specified in the merge-schedule.yml workflow file.

In handle-pull-request.ts it looks like it should reflect the time zone in the comments added, but that doesn't seem to be the case: https://github.com/gr2m/merge-schedule-action/blob/4575b62e8af729a3d4a0f0341d2515ae9d4ab908/lib/handle-pull-request.ts#L71-L88

merge-schedule.yml

name: Merge Schedule

on:
  pull_request:
    types:
      - opened
      - edited
      - synchronize
  schedule:
    # https://crontab.guru/every-hour
    - cron: '*/15 * * * *'

jobs:
  merge_schedule:
    runs-on: ubuntu-latest
    steps:
      - uses: gr2m/merge-schedule-action@v2
        with:
          # Merge method to use. Possible values are merge, squash or
          # rebase. Default is merge.
          merge_method: merge
          # Time zone to use. Default is UTC.
          time_zone: 'Europe/Copenhagen'
          # Require all pull request statuses to be successful before
          # merging. Default is `false`.
          require_statuses_success: 'true'
          # Label to apply to the pull request if the merge fails. Default is
          # `automerge-fail`.
          automerge_fail_label: 'merge-schedule-failed'
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Example of flow in PR image

diegocam commented 1 week ago

This is the problem I'm understanding:

Let's say a user sets up the Github action:

on:
  schedule:
    - cron: "0 * * * *"

jobs:
  steps:
    - uses: gr2m/merge-schedule-action@v2
       with:
         time_zone: "America/New_York"

Problem

The user sets a date/time to be scheduled based on their timezone, at least most would assume this. However, it is registered as UTC. This is the current flow:

  1. The PR is created with body “\schedule 2024-10-25 10:00:00”
  2. A comment is created on the PR as
     Scheduled to be merged on 2024-10-25 10:00:00 (UTC)
  3. When merged, the comment is updated:
    Scheduled on 2024-10-25 10:00:00 (UTC) successfully merged

    Furthermore, when iterating through the opened PRs, here https://github.com/gr2m/merge-schedule-action/blob/678b3399de95e30d1c7c48b319b3b86b533d2ab9/lib/handle-schedule.ts#L70C1-L74C5, we compare PRs with bodies that have strings "\schedule" or "\schedule 2024-10-25T10:00:00", and for those that have a date, we compare them as such:

    new Date(pullRequest.scheduledDate) < localeDate()

    where localDate() does the following:

    function localeDate(): Date {
    const localeString = new Date().toLocaleString("en-US", {
    timeZone: process.env.INPUT_TIME_ZONE,
    });
    return new Date(localeString);
    }

    in other words we are comparing these two date objects:

  4. one based on the dateString using local timezone ("UTC" when Github runs it)
  5. the other based on the local time using the given timezone ("America/New_York" based on the input above)

So for example if the github action runs at 2024-10-25 00:00:00 and the PR is set to “\schedule 2024-10-25T10:00:00.000Z” (10AM), the following dates will be compared:

  1. new Date("2024-10-25T10:00:00.000Z") UTC -> 2024-10-25T05:00:00.000Z
  2. new Date() "America/New_York" -> 2024-10-25T11:00:00.000Z

Why are we comparing these? Maybe im not understanding this concept?


Proposed Solution

User sets a date/time to be scheduled based on their timezone and it is registered using their timezone

  1. The PR is created with body “\schedule 2024-10-25 10:00:00”
  2. A comment is created on the PR as
     Scheduled to be merged on 2024-10-25 10:00:00 (EST)
  3. When merged, the comment is updated:
    Scheduled on 2024-10-25 10:00:00 (EST) successfully merged

When the code compares dates (to see if they should be merged), it should compare [scheduledDate < localDate] both based on the same timezone though, the given timezone by the user.


@gr2m Please let me know if the above makes sense in order to make the proper corrections.

gr2m commented 1 week ago

That sounds great! Timezones are hard so I went the easy way with UTC, but your suggestions are sound and the improvement would be great to have