Skip to content
This repository has been archived by the owner on Oct 1, 2021. It is now read-only.

Initial work #1

Merged
merged 65 commits into from
Oct 17, 2019
Merged

Initial work #1

merged 65 commits into from
Oct 17, 2019

Conversation

AuHau
Copy link
Member

@AuHau AuHau commented Apr 4, 2019

A first draft version of the migration tool, that can be meaningfully feedbacked.
To see how migration looks like, see #2

This PR is written against the async PR's of dependencies (datastores mainly) and hence is blocked by this.

Scope-in

Scope and status of this PR (and package):

  • Define the migration's API
  • Create high-level API to migrate forward&backward migrations
  • Create CLI to run the migrations
  • Write documentation to README
    • Document tests, how to write them, run them etc
  • Write test coverage

Scope-out

Thinks that are not in scope of this PR and will be scope of separate PR to js-ipfs that will integrate this package:

  • Disabling/enabling automatic migrations
  • Propagating information about migrations to JS-IPFS readme
  • Integrate to js-ipfs with posibility to automatically run the migrations

Blocked by

Currently, this depends on async/await refactor of

Problems

  1. If the automatic migrations will be disabled in browser environment, how does the user can manually migrate its repo, when it does not have access to CLI?
  2. If IPFSRepo() is used with custom options (e.g. using datastore-level for key storage backend, instead of default datastore-fs), this is not propagated to migration. ==> Should repo.options be propagated through migrate()/revert() to each migration?
    6dc649b added support for passing options to the migrations
  3. Since now it is possible to pass custom options to migrations, what should be done with the CLI? How to define the options for the datastores? They are not stored in config, if I understand it correctly...

AuHau added 13 commits March 5, 2019 13:53
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
…astore instance.

License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
@AuHau AuHau changed the title [WIP] Initial work Initial work Apr 27, 2019
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
@AuHau
Copy link
Member Author

AuHau commented Apr 28, 2019

So the PR is ready for review. I would also ask for the guidance of what next steps should be (ofc after reviews)? I assume something like:

  • Move repo under IPFS org
  • License file move to Protocol Labs?
  • Integrate CI
  • Release

As a community member, I can't do much of these, but I would love to work with you, so you could just hire me and then I could do them! 😉

Blocked by release of async/await versions of

  • interface-datastore
  • js-datastore-fs
  • js-datastore-level

License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
@AuHau
Copy link
Member Author

AuHau commented Jul 23, 2019

I have also moved the repo under IPFS org. What kind of teams should be assigned to it? I have added JS Team, but not sure if some others should have access also.

Also what about "Lead Maintainer"? I can do it, or if somebody else should be it (because PL guidelines or something), then I don't mind that either.

run.md Outdated Show resolved Hide resolved
run.md Outdated Show resolved Hide resolved
run.md Outdated Show resolved Hide resolved
run.md Outdated Show resolved Hide resolved
run.md Outdated Show resolved Hide resolved
src/commands.js Outdated Show resolved Hide resolved
src/commands.js Outdated Show resolved Hide resolved
src/commands.js Outdated Show resolved Hide resolved
src/commands.js Outdated Show resolved Hide resolved
src/commands.js Outdated Show resolved Hide resolved
AuHau and others added 6 commits July 24, 2019 07:34
Co-Authored-By: dirkmc <dirkmdev@gmail.com>
Co-Authored-By: dirkmc <dirkmdev@gmail.com>
Co-Authored-By: dirkmc <dirkmdev@gmail.com>
src/cli.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved

return true
} catch (e) {
log('While checking if repo is initialized error was thrown: ' + e.message)
Copy link

Choose a reason for hiding this comment

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

Please use explain-error here

src/repo/version.js Show resolved Hide resolved
AuHau and others added 3 commits July 25, 2019 09:45
@dirkmc
Copy link

dirkmc commented Jul 26, 2019

@AuHau thank you again for all your hard work. This is looking really good 👍

@alanshaw I've done a couple of passes over the code and suggested some changes, all of which @AuHau has addressed. I think this is ready for you to review 🚀

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

There is a ton of kickass work here!! thank you so much @AuHau

Let's get it out of the door. I've made some comments on the README. some are nitpicks others are org structure.

indent_size = 2
trim_trailing_whitespace = true
insert_final_newline = true
end_of_line = lf
Copy link
Member

Choose a reason for hiding this comment

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

Mind adding this file to .gitignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well I can, but I think it is a good thing to have around as it unities the code style. I know that for that there is aegir lint but this integrates directly with IDEs. You don't use it in other projects? I feel like I took it from some other JS package...
For example here: https://github.com/ipfs/js-ipfs/blob/master/.editorconfig

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
* `options.ignoreLock` (bool, optional) - if true will not lock the repo when applying migrations. Use with caution.
* `options.repoOptions` (object, optional) - options that are passed to migrations, that use them to construct the datastore. (options are the same as for IPFSRepo).
* `options.onProgress` (function, optional) - callback that is called after finishing execution of each migration to report progress.
Copy link
Member

Choose a reason for hiding this comment

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

Is this for progress bars?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

2 more :)

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-Authored-By: David Dias <daviddias.p@gmail.com>
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/repo/lock.js Show resolved Hide resolved
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.

4 participants