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

tkts parameters are true by default #125

Closed
wants to merge 0 commits into from
Closed

Conversation

brngylni
Copy link
Contributor

This relates to issue #79. Parameters have been made True by default. Parameters themselves and also their descriptions are updated accordingly.

@gridhead gridhead self-requested a review March 19, 2024 04:57
@gridhead gridhead self-assigned this Mar 19, 2024
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

There is apparently some confusion at your end on what the options are supposed to achieve. Please make the requested corrections.

pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
pagure_exporter/main.py Outdated Show resolved Hide resolved
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

@brngylni here is a quick explanation of what was suggested to be done and how it affects the codebase.

@@ -99,7 +99,7 @@ def main(srce, dest, pkey, gkey, fusr, tusr):
"status",
type=click.Choice(["OPEN", "SHUT", "FULL"], case_sensitive=False),
help="Extract issue tickets of the mentioned status",
default="OPEN",
default="FULL",
Copy link
Member

Choose a reason for hiding this comment

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

The option which previously defaulted to transferring only OPEN tickets will now default to transferring tickets regardless of their states.

help="Transfer all the associated comments",
default=False,
help="Don't transfer the associated comments",
default=True,
Copy link
Member

Choose a reason for hiding this comment

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

The option which previously defaulted to not transferring comments alongside the issue tickets will now default to transferring comments too.

help="Migrate all the associated labels",
default=False,
help="Don't migrate the associated labels",
default=True,
Copy link
Member

Choose a reason for hiding this comment

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

The option which previously defaulted to not transferring labels alongside the issue tickets will now default to transferring labels too.

help="Assert issue ticket states as they were",
default=False,
help="Issue ticket states don't have to be in the state they were in",
default=True,
Copy link
Member

Choose a reason for hiding this comment

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

The option which previously defaulted to not transferring issue states alongside the issue tickets will now default to transferring issue states too.

help="Confirm issue ticket privacy as they were",
default=False,
help="Don't account issue ticket privacy",
default=True,
Copy link
Member

Choose a reason for hiding this comment

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

The option which previously defaulted to not transferring issue privacy alongside the issue tickets will now default to transferring issue privacy too.

help="Ensure issue ticket sequence as they were",
default=False,
help="Don't account the issue ticket sequence",
default=True,
Copy link
Member

Choose a reason for hiding this comment

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

The option which previously defaulted to not transferring the sequence of the issue tickets will now default to transferring the sequence too.

Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Please fix the tests while you are at it.

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.

Assert tkts option defaults should include everything unless specified otherwise
2 participants