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

Rewrite doctest as unit test #5818

Merged
merged 2 commits into from
Feb 23, 2024
Merged

Conversation

MetRonnie
Copy link
Member

Closes #4413

This rewrite was handled by GitHub Copilot

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor. (But GitHub Copilot has not added themselves...)
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • No dependency changes
  • Tests
  • Not a change that can affect users
  • Docs not required
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@MetRonnie MetRonnie added this to the cylc-8.2.4 milestone Nov 13, 2023
@MetRonnie MetRonnie self-assigned this Nov 13, 2023
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Not the most suitable doctest, but I don't get the motive for translating it?

@MetRonnie
Copy link
Member Author

To avoid the hacky patching of LOG.warning in the doctest. Why tackle this issue now? Simply to test out the capabilities of Copilot

@oliver-sanders
Copy link
Member

Ah, hadn't spotted the attached issue, sorry, must read better.

@oliver-sanders
Copy link
Member

Blocked by copilot approval?

@MetRonnie
Copy link
Member Author

MetRonnie commented Nov 14, 2023

Blocked by copilot approval?

No, as mentioned on Teams the guidance wasn't clear that Copilot has been approved for this trial.

Anyway, if I was going to manually convert this test the code would have been 100% identical...

@hjoliver hjoliver added the BLOCKED This can't happen until something else does label Nov 27, 2023
@hjoliver
Copy link
Member

(Added blocked label pending approval at your end).

@MetRonnie

This comment was marked as resolved.

@MetRonnie MetRonnie removed the BLOCKED This can't happen until something else does label Nov 28, 2023
@oliver-sanders oliver-sanders added the BLOCKED This can't happen until something else does label Nov 28, 2023
@MetRonnie
Copy link
Member Author

MetRonnie commented Nov 28, 2023

Ok, Oliver wants to take this PR as an opportunity to have the generative AI guidance at our end improved as it's still a bit unworkable in its current draft

@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.4, cylc-8.3.0 Jan 8, 2024
@MetRonnie MetRonnie changed the base branch from 8.2.x to master February 19, 2024 17:23
@MetRonnie MetRonnie modified the milestones: cylc-8.3.0, cylc-8.x Feb 19, 2024
@MetRonnie MetRonnie removed the BLOCKED This can't happen until something else does label Feb 23, 2024
@MetRonnie
Copy link
Member Author

Unblocked

@oliver-sanders oliver-sanders merged commit acb164f into cylc:master Feb 23, 2024
33 checks passed
@MetRonnie MetRonnie deleted the unit-test branch February 23, 2024 13:13
@MetRonnie MetRonnie modified the milestones: cylc-8.x, cylc-8.3.0 Feb 23, 2024
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.

Replace cylc.flow.timer doctests with unit tests
3 participants