Skip to content
This repository has been archived by the owner on Sep 6, 2024. It is now read-only.

Matt/collab/onfront 73 #41

Merged
merged 14 commits into from
Jul 15, 2024
Merged

Matt/collab/onfront 73 #41

merged 14 commits into from
Jul 15, 2024

Conversation

kyle-binger-3
Copy link
Collaborator

This PR implements changes to the ontario-add-package and ontario-remove-package commands. Both now accept multiple package values (through a question prompt showing only our https://github.com/ongov packages) as their respective arguments.

Along with this, there are some other minor changes:

  • Logging has been improved for these commands
  • isPackageInstalled() now also checks devDependencies within the generated project's package.json
  • A new package util checkAndRemoveConfigFiles() has been added and is in use during the execution of ontario-remove-package
  • A new datastructure util has been created for parsing object key/values into an array

Copy link
Contributor

@syed-ods syed-ods left a comment

Choose a reason for hiding this comment

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

LGTM. Checked all the cases mentioned in the acceptance criteria and everything is working as expected.

Some observations:

  • In the CLI checkbox options, the <a> and <i> key options are doing the same thing: toggling back and forth between select and unselect/deselect all. We can keep one key only and adjust the message.
  • Removing packages that are not installed:
    If I try to remove packages that are not installed/uinstalled, the process should exit before giving the confirmation prompt. In my opinion, there is no need for that prompt if the packages don't exist, because even with the confirmation prompt, we'll be giving the same response/log messages after that extra step.
  • The tab key should also work in addition to the arrow keys while selecting the packages through the CLI.

Again, great work, and sorry for my nit-picky observations.

Copy link
Member

@matt-stjean matt-stjean left a comment

Choose a reason for hiding this comment

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

A lot of my comments and feedback were given through slack and this branch. The various scenarios work locally for me and this set up is really slick, good job

kyle-binger-3 and others added 14 commits July 15, 2024 10:46
 - addPackage question now displays checkbox list of available packages to install
 - eslint and prettier config object keys renamed to reflect the @ongov package names
 - `isPackageInstalled()` now also checks for packages within `devDependencies`
 - `extractChildObjectValuesByKey()` util now has graceful failure and descriptive comments
- removed redundant `isOntarioProject` check from addPackage command
   - same check is found with `ontario-add-package` bin file
- standardized debug messages across add and remove package commands
@kyle-binger-3 kyle-binger-3 merged commit b2742e4 into develop Jul 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants