Skip to content
This repository was archived by the owner on Jul 11, 2026. It is now read-only.

PR - Mónica#52

Open
ClaviHaze wants to merge 3 commits into
Laboratoria:masterfrom
ClaviHaze:master
Open

PR - Mónica#52
ClaviHaze wants to merge 3 commits into
Laboratoria:masterfrom
ClaviHaze:master

Conversation

@ClaviHaze

Copy link
Copy Markdown

No description provided.

@ClaviHaze ClaviHaze changed the title Mónica Rodríguez Pull Request PR - Mónica Jul 13, 2020
Comment thread app.js
if (file) {

} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cuál era tu idea aquí? Cuál era el siguiente paso y el objetivo de esta función?

Comment thread app.js
}];
}
return
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Acá cuál era el objetivo? que iba a devolver si solo era un archivo?

Comment thread app.js

const isMarkDown = (file) => {
const fileExt = path.extname(uri);
console.log(fileExt)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por favor elimina los console innecesarios.

Comment thread app.js
const linkStatus = (links) => {

};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cuál era el objetivo de estas dos funciones?

Comment thread app.js

const extractLinks = (data) => {
const getLinks = /((https?:\/\/)|(http?:\/\/)|(www\.))[^\s\n)]+/g;
const getLinkName = /(?:[^[])([^[]*)(?=(\]+\(((https?:\/\/)|(http?:\/\/)|(www\.))))/g;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por qué tenemos dos regex? para qué son?

Comment thread test/app.spec.js
it('return true if file exists', () => {
expect(mdlink.isFile(testdir)).toBe(false);
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por qué tenemos dos pruebas diferentes? Me parece que hacen lo mismo

Comment thread test/app.spec.js
expect(mdlink.getFile(testdir)).toBe('./test/testdir/innertest.md');
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Muy buenas pruebas!

Comment thread utils/validate.js
)
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por qué separaste esta función del otro archivo?

Comment thread app.js
return false;
};
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Me gustan mucho estas dos funciones! me gusta tu approach :)

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.

2 participants