Skip to content

Conversation

@lukaszb
Copy link

@lukaszb lukaszb commented Jan 2, 2018

boostnote-init-modal-label

It was strange for me to open the app on another machine, select existing directory I want to use and seeing "Create" label. I was afraid it would override my storage created on another machine (lying within "cloud drive").

Have two questions, though:

  1. I'd also update same thing at the storages tab but am not sure if you prefer two smaller PRs or one with both changes
  2. Do we have any common place for utils like pathExists functions? I've left it at InitModal component but would prefer to extract it somewhere (as this function would be used for similar check at storages tab)

@kazup01 kazup01 requested a review from sosukesuzuki January 3, 2018 18:40
@kazup01 kazup01 added the awaiting review ❇️ Pull request is awaiting a review. label Jan 3, 2018
@sosukesuzuki
Copy link
Member

sosukesuzuki commented Jan 4, 2018

I answer your two questions.

  1. I prefer two smaller PR, because it is easy to review for me. However, if you prefer one with both changes PR, it does not matter.
  2. Common place for utils is ./browser/lib

Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

please fix from CI.

@lukaszb
Copy link
Author

lukaszb commented Jan 4, 2018

@sosukesuzuki all right, thanks. I'll fix lint errors and put pathExists function under ./browser/lib and ping you back. Will create another PR after this one is merged.

@kazup01 kazup01 added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Jan 5, 2018
@lukaszb lukaszb closed this Jan 5, 2018
@lukaszb
Copy link
Author

lukaszb commented Jan 5, 2018

Reopening to notify CI to re-run tests - it seems there is some randomly failing test.

@lukaszb lukaszb reopened this Jan 5, 2018
@lukaszb
Copy link
Author

lukaszb commented Jan 5, 2018

@sosukesuzuki done

@sosukesuzuki
Copy link
Member

Very sorry, for we can't merge your PR 🙇
We selected the way that removing InitModal and instead it Boostnote create the folder named "Boostnote" in ~/Boostnote. (#1379)
However, we are always waiting for your contribution.:+1:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants