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

feat: remove 'no' prefix in --no-progress and --no-install-fonts CLI flags #322 #325

Merged

Conversation

CAMOBAP
Copy link
Contributor

@CAMOBAP CAMOBAP commented Mar 31, 2024

@@ -64,7 +64,7 @@ def compile(file_name = nil)
option :coverpage, aliases: "-c", desc: "Liquid template"
option :agree_to_terms, type: :boolean, desc: "Agree / Disagree with all third-party licensing terms "\
"presented (WARNING: do know what you are agreeing with!)"
option :no_install_fonts, type: :boolean, desc: "Skip the font installation process"
option :install_fonts, type: :boolean, default: true, desc: "Install required fonts"
Copy link

Choose a reason for hiding this comment

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

Line is too long. [90/80]

@@ -39,7 +39,7 @@ def new(name)
option :strict, aliases: "-S", type: :boolean, desc: "Strict compilation: abort if there are any errors"
option :agree_to_terms, type: :boolean, desc: "Agree / Disagree with all third-party licensing terms "\
"presented (WARNING: do know what you are agreeing with!)"
option :no_install_fonts, type: :boolean, desc: "Skip the font installation process"
option :install_fonts, type: :boolean, default: true, desc: "Install required fonts"
Copy link

Choose a reason for hiding this comment

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

Line is too long. [90/80]

class_option :no_progress, aliases: "-s", type: :boolean, default: true,
desc: "Don't show progress for long running tasks (like download)"
class_option :progress, aliases: "-s", type: :boolean, default: false,
desc: "Show progress for long running tasks (like download)"
Copy link

Choose a reason for hiding this comment

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

Line is too long. [90/80]

@@ -18,7 +18,7 @@ class Site < ThorWithConfig
)
option :agree_to_terms, type: :boolean, desc: "Agree / Disagree with all third-party licensing terms "\
"presented (WARNING: do know what you are agreeing with!)"
option :no_install_fonts, type: :boolean, desc: "Skip the font installation process"
option :install_fonts, type: :boolean, default: true, desc: "Install required fonts"
Copy link

Choose a reason for hiding this comment

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

Line is too long. [92/80]

@ronaldtse
Copy link
Contributor

This proposed change is fine, but we must have a proper announcement / documentation change.

@CAMOBAP
Copy link
Contributor Author

CAMOBAP commented Mar 31, 2024

This proposed change is fine, but we must have a proper announcement / documentation change.

I will prepare a blog post PR for metanorma.com

@CAMOBAP CAMOBAP force-pushed the feature/remove-no-prefix-in-progress-and-install-fonts-322 branch from 0fad915 to 9d88e6f Compare April 24, 2024 19:47
@opoudjis
Copy link
Contributor

opoudjis commented May 6, 2024

@CAMOBAP Is this ready to review yet?

@CAMOBAP CAMOBAP force-pushed the feature/remove-no-prefix-in-progress-and-install-fonts-322 branch 2 times, most recently from 15a4436 to a7529b5 Compare May 24, 2024 14:18
@CAMOBAP CAMOBAP force-pushed the feature/remove-no-prefix-in-progress-and-install-fonts-322 branch from a7529b5 to 1537ba1 Compare May 24, 2024 14:57
@CAMOBAP CAMOBAP marked this pull request as ready for review May 24, 2024 19:58
@CAMOBAP
Copy link
Contributor Author

CAMOBAP commented May 25, 2024

https://github.com/metanorma/metanorma-cli/actions/runs/9225953709 - is passed, I'm rollback Gemfile.devel

@CAMOBAP CAMOBAP force-pushed the feature/remove-no-prefix-in-progress-and-install-fonts-322 branch from 1537ba1 to 5797b26 Compare May 25, 2024 13:24
@CAMOBAP
Copy link
Contributor Author

CAMOBAP commented May 28, 2024

@opoudjis to be on the same page this PR is ready for review/merge. tests passed during https://github.com/metanorma/metanorma-cli/actions/runs/9225953709 run

And metanorma/ci#167 has to be merged right before this one.

Feel free to merge yourself

@opoudjis opoudjis merged commit e537533 into main May 28, 2024
9 of 21 checks passed
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.

None yet

3 participants