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.

Sign up for GitHub

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

Jump to bottom

Consistent xodc CLI #1518

Merged
merged 12 commits into from
Nov 6, 2018
Merged

Consistent xodc CLI #1518

merged 12 commits into from
Nov 6, 2018

Conversation

knopki
Copy link
Member

@knopki knopki commented Oct 30, 2018 •
edited
Loading

Full reimplementation of xodc with oclif framework as first part of #884.
Implemented commands: resave, tabtest, transpile, publish. See README.md.
Notable change in publish command: reimplement api calls from swagger-client and old orgs api, to new users api with plain fetch requests.
Not implemented: correct workspace creation because of arduino-cli rework needed.

Also, closes #1517 - wrong typings in xod-tabtest. Thanks to @evgenykochetkov

@knopki knopki self-assigned this Oct 30, 2018
@knopki knopki requested review from evgenykochetkov, brusherru and nkrkv and removed request for evgenykochetkov and brusherru October 30, 2018 10:55
brusherru
brusherru requested changes Oct 30, 2018
View reviewed changes
Copy link
Contributor

@brusherru brusherru left a comment

Choose a reason for hiding this comment

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

Left some comments. But did not tested yet.

packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/src/args.js Outdated Show resolved Hide resolved
packages/xod-cli/src/baseCommand.js Outdated Show resolved Hide resolved
packages/xod-cli/src/commands/publish.js Outdated Show resolved Hide resolved
packages/xod-cli/src/commands/tabtest.js Show resolved Hide resolved
packages/xod-cli/src/commands/tabtest.js Outdated Show resolved Hide resolved
packages/xod-cli/src/messages.js Outdated Show resolved Hide resolved
@knopki
Copy link
Member Author

knopki commented Oct 30, 2018

Also, I want to move coerceWorkspace and may be some other functions to BaseCommand as advised by @brusherru

@knopki knopki added the wip label Oct 30, 2018
@knopki knopki force-pushed the feat-884-consistent-xodc branch 4 times, most recently from 2b0c348 to 7c2a966 Compare October 31, 2018 02:59
@knopki knopki removed the wip label Oct 31, 2018
@knopki
Copy link
Member Author

knopki commented Oct 31, 2018

@brusherru Please, review. Many new bugs invented!

nkrkv
nkrkv requested changes Oct 31, 2018
View reviewed changes
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

The CI script should be adjusted as well so that tabtests start to use the new machinery right after the merge.

packages/xod-tabtest/src/Tabtest.re Show resolved Hide resolved
packages/xod-cli/.gitignore Outdated Show resolved Hide resolved
packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/README.md Show resolved Hide resolved
packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/src/commands/publish.js Outdated Show resolved Hide resolved
packages/xod-cli/src/constants.js Outdated Show resolved Hide resolved
packages/xod-cli/src/flags.js Outdated Show resolved Hide resolved
packages/xod-cli/src/flags.js Outdated Show resolved Hide resolved
packages/xod-cli/src/flags.js Outdated Show resolved Hide resolved
@knopki knopki force-pushed the feat-884-consistent-xodc branch 3 times, most recently from b4b138c to 9a1752e Compare November 5, 2018 01:30
@knopki
Copy link
Member Author

knopki commented Nov 6, 2018

+ listr stack with nice messages and real concurency (like in publish command), createError/formatters/nice error template, optimized yarn.lock impact, tests for tty/notty
- drop all old messages and related output code

@nkrkv @brusherru roast me.

nkrkv
nkrkv requested changes Nov 6, 2018
View reviewed changes
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

Very nice 😎

I have few notes regarding wording/spelling though.

packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/README.md Outdated Show resolved Hide resolved
packages/xod-cli/README.md Show resolved Hide resolved
packages/xod-cli/src/messages.js Outdated Show resolved Hide resolved
packages/xod-cli/src/messages.js Outdated Show resolved Hide resolved
packages/xod-cli/src/messages.js Outdated Show resolved Hide resolved
packages/xod-cli/src/messages.js Outdated Show resolved Hide resolved
packages/xod-cli/src/messages.js Outdated Show resolved Hide resolved
@knopki
Copy link
Member Author

knopki commented Nov 6, 2018

@nkrkv updated.

nkrkv
nkrkv approved these changes Nov 6, 2018
View reviewed changes
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

❤️

@knopki knopki merged commit 1c719a5 into master Nov 6, 2018
@knopki knopki deleted the feat-884-consistent-xodc branch November 6, 2018 17:14
@evgenykochetkov evgenykochetkov mentioned this pull request Dec 26, 2018
Merged
@nkrkv nkrkv mentioned this pull request Dec 27, 2018
Closed
This was referenced Aug 1, 2022
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brusherru brusherru brusherru approved these changes

@nkrkv nkrkv nkrkv approved these changes

@evgenykochetkov evgenykochetkov Awaiting requested review from evgenykochetkov

Assignees

@knopki knopki

Labels
s:cli t:feat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

xod-tabtest typing error
3 participants
@knopki @nkrkv @brusherru