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

fix: remove oclif framework and fixes an issue with sites:create #3717

Merged
merged 42 commits into from
Dec 15, 2021

Conversation

lukasholzer
Copy link
Collaborator

@lukasholzer lukasholzer commented Nov 26, 2021

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes https://github.com/netlify/pod-workflow/issues/343

Fixes #3716

Good to know when reviewing

  • commanders syntax for specifying a value on an option or argument tells if it is required or not:
    .option('--data [data]') brackets make data optional can be undefined as well
    .option('--data <data>') angle brackets make passing a data required.
  • checkout the updated docs, as they changed a little bit (best place to identify changes)

Refactorings:

  1. Refactored each command into its own folder
    • Each command (parent or sub) has its own file and exports a create...Command function that has one parameter the program and returns one.
    • Each command folder has a barrel file (index.js) that exports the create....Command function of the main command (not the sub ones) and if needed the action (like in the init case where the init command is reused).
  2. The BaseCommand and the Telemetry are now merged in one class src/commands/base-command.js and every command extends from it.
  3. replaced most src/lib/fs.js methods like readFileAsync through fs.promises.readFile as there is no need to promisify them.
  4. The completion is completely written up from scratch as the old one was a plugin for oclif

Commands to refactor:

  • main entry with update notifier
  • api
  • addons (Not sure on how to test them manually)
    • auth
    • config
    • create
    • delete
    • list
  • build
  • completion
  • deploy
  • dev
    • exec
    • trace
  • env
    • get
    • import
    • list
    • set
    • unset
  • functions
    • build
    • create
    • invoke
    • list
    • serve
  • help
  • init
  • link
  • lm
    • info
    • install
    • setup
    • uninstall
  • login
  • logout
  • open
  • sites
    • create
    • delete
    • list
  • status
    • hooks
  • switch
  • unlink
  • watch
  • base command
  • tracking base

Other things to do

  • create other styling for default help page to be visually the same like the oclif one
  • Fix the oclif error, warning and exit things
  • override the exit and do the send tracking there
  • add debug log scopes
  • check for open todos in code // TODO: ....

image

@github-actions
Copy link

github-actions bot commented Nov 26, 2021

📊 Benchmark results

Comparing with f196c10

Package size: 354 MB

⬇️ 2.56% decrease vs. f196c10

^  363 MB  363 MB  363 MB  363 MB  363 MB  363 MB  363 MB  363 MB  363 MB         
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐   354 MB 
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@lukasholzer lukasholzer added the type: chore work needed to keep the product and development running smoothly label Dec 3, 2021
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Hi @lukasholzer, this is great work.

I left some comments and keep doing so, but I'll let you go over the existing ones while I continue the review to unblock you.

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
site/scripts/generate-command-data.js Show resolved Hide resolved
src/commands/base-command.js Show resolved Hide resolved
erezrokah
erezrokah previously approved these changes Dec 15, 2021
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

I think this is good to go @lukasholzer 🚀

tools/eslint-rules/index.js Show resolved Hide resolved
@lukasholzer lukasholzer changed the title chore: remove oclif fix: Remove oclif framework and fixes an issue with sites:create Dec 15, 2021
@github-actions github-actions bot added the type: bug code to address defects in shipped code label Dec 15, 2021
@lukasholzer lukasholzer removed the type: chore work needed to keep the product and development running smoothly label Dec 15, 2021
@lukasholzer lukasholzer merged commit a920292 into main Dec 15, 2021
@lukasholzer lukasholzer deleted the chore/remove-oclif branch December 15, 2021 11:35
@lukasholzer lukasholzer changed the title fix: Remove oclif framework and fixes an issue with sites:create fix: remove oclif framework and fixes an issue with sites:create Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove oclif and replace with commander
2 participants