Skip to content

Conversation

@guimachiavelli
Copy link
Member

Closes #2079

@guimachiavelli guimachiavelli linked an issue Jan 17, 2023 that may be closed by this pull request
2 tasks
@guimachiavelli guimachiavelli linked an issue Jan 17, 2023 that may be closed by this pull request
@guimachiavelli guimachiavelli marked this pull request as ready for review January 18, 2023 11:48
Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I have a few comments about the unique behavior of the flag.

@guimachiavelli
Copy link
Member Author

@dureuill: thanks for the review! I have tried addressing your comments in two ways:

  1. Having a longer "Default value" to reflect the different ways of using this particular option;
  2. Specifying booleans are only valid when using the config file. Seems like something user relying on automated workflows might like to know.

Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

Thanks @guimachiavelli ! this works for me!

Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

Do you think we should mention disabling snapshots in learn/advanced/snapshots.md? It is a kind of guide, so it would be helpful to have everything snapshot-related mentioned there.

guimachiavelli and others added 3 commits January 25, 2023 12:18
Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>
Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

🐙

@guimachiavelli guimachiavelli added this to the v1.0 milestone Jan 30, 2023
@guimachiavelli
Copy link
Member Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 2, 2023

Build succeeded:

@bors bors bot merged commit 079ae18 into v1.0 Feb 2, 2023
@bors bors bot deleted the v1-schedule-snapshot branch February 2, 2023 17:04
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.

v1.0: change the behavior of --schedule-snapshot v1.0: update telemetry table

4 participants