Skip to content

Conversation

@ehhc
Copy link
Contributor

@ehhc ehhc commented May 23, 2018

I've only inserted an additional IF..

@kazup01 kazup01 added the awaiting review ❇️ Pull request is awaiting a review. label May 24, 2018
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 confirm my review!

if (attachmentsInNote) {
for (let i = 0; i < attachmentsInNote.length; i++) {
attachmentsInNoteOnlyFileNames.push(attachmentsInNote[i].replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + noteKey + escapeStringRegexp(path.sep), 'g'), ''))
if (storageKey && noteKey && markdownContent !== null && typeof markdownContent !== 'undefined') {
Copy link
Member

Choose a reason for hiding this comment

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

please write like below:

if (storageKey != null && noteKey != null && markdownContent != null)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right and i've fixed the issue like you suggested

@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 May 28, 2018
@ehhc
Copy link
Contributor Author

ehhc commented May 28, 2018

@sosukesuzuki i've changed it like you said ;)

@kazup01 kazup01 added awaiting review ❇️ Pull request is awaiting a review. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels May 28, 2018
if (attachmentsInNote) {
for (let i = 0; i < attachmentsInNote.length; i++) {
attachmentsInNoteOnlyFileNames.push(attachmentsInNote[i].replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + noteKey + escapeStringRegexp(path.sep), 'g'), ''))
if (storageKey != null && noteKey != null && markdownContent != null) {
Copy link
Member

Choose a reason for hiding this comment

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

It might be better for this to be able to reduce nesting, like below:

if (storageKey == null || noteKey == null || markdownContent == null) return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@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 Jun 5, 2018
for (let i = 0; i < attachmentsInNote.length; i++) {
attachmentsInNoteOnlyFileNames.push(attachmentsInNote[i].replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + noteKey + escapeStringRegexp(path.sep), 'g'), ''))
}
if (storageKey == null || noteKey == null || markdownContent == null) {
Copy link
Member

Choose a reason for hiding this comment

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

{} is unnecessary.please write like below:

if (storageKey == null || noteKey == null || markdownContent == null) return

@Rokt33r Rokt33r added next release (v0.11.6) and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Jun 6, 2018
@Rokt33r Rokt33r merged commit 88856b7 into BoostIO:master Jun 6, 2018
@ehhc ehhc deleted the OnBlur_Throws_Exceptions_On_Snippet_Notes branch June 6, 2018 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants