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

Rename CLI options #111

Open
jpmckinney opened this issue May 28, 2021 · 5 comments
Open

Rename CLI options #111

jpmckinney opened this issue May 28, 2021 · 5 comments
Labels
cli CLI changes that have limited impact on library code

Comments

@jpmckinney
Copy link
Member

jpmckinney commented May 28, 2021

Some option names could perhaps be improved: https://open-contracting.github.io/spoonbill/cli.html

@sabahfromlondon @yolile What do you think of?

  • --threshold NUM -> --split-at NUM, which communicates what action happens at the threshold
  • --selection TABLE,TABLE -> --only-initial-tables TABLE,TABLE, which clarifies that this is only for initial tables (described in the docs as the top-level planning, tenders, awards, contracts tables), not child tables or columns
  • --exclude TABLE,TABLE -> --exclude-child-tables TABLE,TABLE, which clarifies that this is only for child tables, not initial tables or columns

I think all other option names are clear (most options are for columns, so I think okay to not have a -columns suffix).

@jpmckinney jpmckinney self-assigned this May 28, 2021
@sabahfromlondon
Copy link

@jpmckinney I like the suggested changes a lot. split-at NUM for instance is much clearer.

The only suggestion I have is that maybe instead of initial tables use the wording root-tables?

@yolile
Copy link
Member

yolile commented Jun 1, 2021

I agree with @sabahfromlondon, and another option instead of initial tables is "parent" tables, which makes sense as we are using "child-tables" later.

@sorenabell sorenabell added this to the Milestone 3.0 milestone Jun 3, 2021
@jpmckinney
Copy link
Member Author

I'm happy with --only-root-tables. --only-parent-tables could be confusing, because contracts_items is a parent table to contracts_items_class, but it is not an initial/root table.

@jpmckinney
Copy link
Member Author

@sorenabell This is the sort of small change that I am happy to commit on my own, so that Quinta can focus on the more challenging and time-consuming changes.

For this kind of change, do you prefer that I assign the PR for review by Quinta, or just merge it myself?

@sorenabell
Copy link

@jpmckinney please assign PR review for @yshalenyk nevertheless, just for us to be in the loop.

@jpmckinney jpmckinney removed this from the 3.0 milestone Dec 15, 2021
@jpmckinney jpmckinney added documentation Improvements or additions to documentation cli CLI changes that have limited impact on library code and removed documentation Improvements or additions to documentation labels Dec 15, 2021
@jpmckinney jpmckinney changed the title CLI option names: Review Rename CLI option names Dec 15, 2021
@jpmckinney jpmckinney changed the title Rename CLI option names Rename CLI options Dec 15, 2021
@jpmckinney jpmckinney removed their assignment May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli CLI changes that have limited impact on library code
Projects
None yet
Development

No branches or pull requests

4 participants