Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented Aug 3, 2023

@ mention of reviewers

@Didayolo

A brief description of the purpose of the changes contained in this PR.

Now in both V1 and V2 bundles, Phase 1 end and Phase 2 start dates cannot be same

What will happen:

  1. V1 if a phase has no end date and this phase is not the last phase, use the next phase (start date - 1) as its end date
  2. V2: check if a phase is not the first phase, and its start date and previous phase end date are same, then raise error

V2 error screenshot

V2

Issues this PR resolves

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@Didayolo
Copy link
Member

Didayolo commented Aug 9, 2023

@ihsaan-ullah Thank you for the PR.

Phase 1 end and Phase 2 start dates cannot be same

But then how to have the Phase 2 starting exactly when the Phase 1 is over? If P1 ends January 5, and P2 starts January 6, will the phase change operates at midnight between January 5 and 6?

@ihsaan-ullah
Copy link
Collaborator Author

@ihsaan-ullah Thank you for the PR.

Phase 1 end and Phase 2 start dates cannot be same

But then how to have the Phase 2 starting exactly when the Phase 1 is over? If P1 ends January 5, and P2 starts January 6, will the phase change operates at midnight between January 5 and 6?

Yes, When on server date changes to 6 January (i.e. after 23:59:50 on January 5), Phase 1 will end and phase 2 will start

@Didayolo
Copy link
Member

Didayolo commented Aug 9, 2023

OK, I'll check that this is not breaking old bundles and on-going benchmarks.

@Didayolo Didayolo merged commit 819087f into develop Aug 10, 2023
@Didayolo Didayolo deleted the phase_dates branch August 10, 2023 15:01
@Didayolo Didayolo mentioned this pull request Aug 10, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants