-
Notifications
You must be signed in to change notification settings - Fork 18
feat(cli): rename cogify package to cli-raster BM-1262 #3433
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
Conversation
Looks good to me. Just one thought: @Wentao-Kuang - Are we wanting to keep the sub-command as |
I think cogify looks good to me. |
@Wentao-Kuang - I need to build this pull request as a container and make some adjustments in the topo-workflows repository. Once all is working on my end, I'll approve this work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ccbblin - Noticed a few things from my container testing:
-
Now that we've changed the package name, we should update the
packageName
parameter passed to thegetLogger
function:// before const logger = getLogger(this, args, 'cogify'); // after const logger = getLogger(this, args, 'cli-raster');
Instances:
const logger = getLogger(this, args, 'cogify'); const logger = getLogger(this, args, 'cogify'); const logger = getLogger(this, args, 'cogify'); const < 8000 span class="pl-s1">logger = getLogger(this, args, 'cogify');
-
Lerna highlights a circular dependency between the
cli
andcli-raster
packages:The
cli-raster
package doesn't actually depend on thecli
package. Verified this withdepcheck
. We should remove the followingdevDependencies
entry:basemaps/packages/cli-raster/package.json
Line 43 in ce33659
"@basemaps/cli": "^7.16.0",
# [v8.0.0](v7.17.0...v8.0.0) **Please Note this is a work in progress release, please remain on v7 until the command line changes have been finalized** ### Bug Fixes * **cli:** update landing deploy script with new cli-config package BM-1260 ([#3431](#3431)) ([4d7e86d](4d7e86d)), closes [/github.com/linz/basemaps/blob/e527a04fec65c82f8577642493e8fcb475762243/packages/landing/scripts/deploy.mjs#L1](https://github.com//github.com/linz/basemaps/blob/e527a04fec65c82f8577642493e8fcb475762243/packages/landing/scripts/deploy.mjs/issues/L1) * ensure all linzjs packages are correctly labeled as deps ([#3439](#3439)) ([de9df87](de9df87)), closes [#3438](#3438) * ensure api key blocks are set in ci/cd ([#3442](#3442)) ([28f6403](28f6403)) ### Features * **cli-vector:** Extract cli to load schema json and prepare jobs to process vector mbtiles. BM-1267 ([#3429](#3429)) ([db113e2](db113e2)) * **cli:** add cli-config package BM-1260 ([#3428](#3428)) ([4ca5a47](4ca5a47)) * **cli:** move cogify create-config into cli-config package BM-1261 ([#3432](#3432)) ([5f72430](5f72430)) * **cli:** rename cogify package to cli-raster BM-1262 ([#3433](#3433)) ([36d4449](36d4449)) * **cli:** update cli-package commands to cmd-ts BM-1259 ([#3427](#3427)) ([46cc342](46cc342))
Motivation
We are in the process of migrating our Vector ETL system into Basemaps as a new package. To prepare for this migration, we are undertaking some housekeeping work (BM-1214). This pull request focuses on renaming cogify package to cli-raster.
Modifications
packages/cogify
topackages/cli-raster
Verification
check cli/bin/bmc.js cogify runs correctly