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

Move js & js-old code to github.com/parity-js #7685

Merged
merged 6 commits into from
Feb 5, 2018
Merged

Move js & js-old code to github.com/parity-js #7685

merged 6 commits into from
Feb 5, 2018

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 24, 2018

So long and thanks for all the fish.

  • Move js code to github.com/parity-js into separate repos
  • Updated shell & wallet references with cargo update -p parity-ui
  • Tested operation of UI with cargo build --release --no-default-features --features ui
  • Update CI scripts to remove js-build & js-release

cc @amaurymartiny @ngotchac @tomusdrw

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 24, 2018
@jacogr jacogr added this to the 1.10 milestone Jan 24, 2018
@jacogr jacogr requested a review from tomusdrw January 24, 2018 16:08
Cargo.lock Outdated
@@ -2214,13 +2214,15 @@ dependencies = [
[[package]]
name = "parity-ui-dev"
version = "1.9.0"
source = "git+https://github.com/parity-js/shell.git?rev=c4ad493b86fd8dda0364881d9cff39c95e2ee73b#c4ad493b86fd8dda0364881d9cff39c95e2ee73b"
Copy link
Contributor Author

@jacogr jacogr Jan 24, 2018

Choose a reason for hiding this comment

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

This auto-gen line looks funny. (And the next one - not sure how/why there are 2 refs? Or rather what I assume to be refs)

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's because you have set explicit rev in Cargo.toml file. Instead you could just set up github repo and let Cargo.lock maintain the revision.
But it's fine, might be a bit more pain in the ass to update though:

1. edit Cargo.toml
2. Run $ cargo update -p parity-ui-dev

vs

$ cargo update -p parity-ui-dev

Copy link
Contributor Author

@jacogr jacogr Jan 25, 2018

Choose a reason for hiding this comment

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

100%, bit more of a pain with the revs - this does feel safer since the shell will morph as we get closer to having it dapp-able.

(e.g. @amaurymartiny is working really hard on allowing the shell to be able to run as a dapp & Electron app #next branch in that repo, if that goes to main it should already be fully removed from the 1.10 binary, but if not it is just easier to manage having a compatible know version based on rev)

Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good, how do we plan to update the revisions though?

Cargo.lock Outdated
@@ -2214,13 +2214,15 @@ dependencies = [
[[package]]
name = "parity-ui-dev"
version = "1.9.0"
source = "git+https://github.com/parity-js/shell.git?rev=c4ad493b86fd8dda0364881d9cff39c95e2ee73b#c4ad493b86fd8dda0364881d9cff39c95e2ee73b"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's because you have set explicit rev in Cargo.toml file. Instead you could just set up github repo and let Cargo.lock maintain the revision.
But it's fine, might be a bit more pain in the ass to update though:

1. edit Cargo.toml
2. Run $ cargo update -p parity-ui-dev

vs

$ cargo update -p parity-ui-dev

@jacogr
Copy link
Contributor Author

jacogr commented Jan 25, 2018

Updating will have to be done manually, i.e. like we do for other packages anyway.

We can still push from the Travis build for the shell & wallet to update the refs, but since we want to not have Parity (1.10) build the UI in, I would rather just update manually when we need to since it is not long-term. (But if need be, can just push from CI again)

@debris debris mentioned this pull request Feb 4, 2018
@debris
Copy link
Collaborator

debris commented Feb 4, 2018

I have no idea why 'js' report is requried. @5chdn is it because of our github repo settings?

@jacogr
Copy link
Contributor Author

jacogr commented Feb 5, 2018

@debris it was changed on the repo settings by @5chdn - should be fine now.

@debris debris merged commit 97a3c6e into master Feb 5, 2018
@debris debris deleted the jg-js-repos branch February 5, 2018 09:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants