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

Handle package indexes update fails #1509

Merged
merged 3 commits into from
Nov 2, 2018
Merged

Conversation

brusherru
Copy link
Contributor

Now it will show an error message and remove new URLs from .cli-config.yml that could cause errors (but left downloaded files in the __packages__ directory to make possible to inspect what is really went wrong).
So IDE will be usable after the error.

It fixes #1506

@brusherru brusherru self-assigned this Oct 26, 2018
@brusherru brusherru requested a review from a team October 26, 2018 11:45
@brusherru brusherru changed the base branch from master to 0.25.x October 26, 2018 11:45
@nkrkv nkrkv changed the title Handle failed updating package indexes Handle package indexes update fails Oct 26, 2018
@brusherru
Copy link
Contributor Author

@nkrkv updated. Check it out, please!

@nkrkv nkrkv added the wip label Oct 30, 2018
@brusherru brusherru removed the wip label Oct 31, 2018
@brusherru
Copy link
Contributor Author

@nkrkv check it out, please!

packages/arduino-cli/src/config.js Outdated Show resolved Hide resolved
packages/arduino-cli/src/config.js Outdated Show resolved Hide resolved
packages/arduino-cli/src/config.js Outdated Show resolved Hide resolved
packages/arduino-cli/src/config.js Outdated Show resolved Hide resolved
@brusherru
Copy link
Contributor Author

@evgenykochetkov fixed! Check it out, please

Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

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

Tested it on macOS.

  • It seems to not work if extra.txt has CRLF line endings
  • The error message seems quite verbose:
    screenshot 2018-10-31 at 18 00 22

@brusherru brusherru added the wip label Oct 31, 2018
@brusherru brusherru removed the wip label Nov 1, 2018
@brusherru
Copy link
Contributor Author

@evgenykochetkov fixed! Check it out, please

@brusherru
Copy link
Contributor Author

@nkrkv check it out, please :)

Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

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

Works like a charm 👍🏻

@brusherru brusherru force-pushed the fix-1506-silent-update-index branch 2 times, most recently from 32c0a33 to 6c10fea Compare November 1, 2018 18:57
@brusherru
Copy link
Contributor Author

brusherru commented Nov 1, 2018

@evgenykochetkov @nkrkv check it out, please!
I think it's much better now :)

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.

😢 Just a straightforward test: checkout, build, start, Deploy → Upload:

Package index broken
Error: null does not have a method named "split"
Check correctness of the corresponding URL in "/home/nailxx/devel/xod-workspace/__packages__/extra.txt" and try to update indexes again

Yes, I have an empty extra.txt like most of the current users are.

packages/xod-client-electron/src/upload/messages.js Outdated Show resolved Hide resolved
packages/xod-client-electron/src/upload/messages.js Outdated Show resolved Hide resolved
packages/arduino-cli/src/listAvailableBoards.js Outdated Show resolved Hide resolved
packages/arduino-cli/src/listAvailableBoards.js Outdated Show resolved Hide resolved
@nkrkv nkrkv added the wip label Nov 2, 2018
…xUrls` (do not mutate) and list not installed boards using URLs from `.cli-config.yml` (as arduino-cli does)
…nds that could fail updating package indexes
@brusherru brusherru removed the wip label Nov 2, 2018
…ting and place bundled package urls in the `extra.txt`
@brusherru
Copy link
Contributor Author

@nkrkv fixed!

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.

Oof, finally, LGTM

@brusherru brusherru merged commit cb9ff42 into 0.25.x Nov 2, 2018
@brusherru brusherru deleted the fix-1506-silent-update-index branch November 2, 2018 13:39
@nkrkv nkrkv added the hotfix Issue/PR for a patch release label Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix Issue/PR for a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants