search

GroceriStar / groceristar-fetch

json arrays for Food Tech projects
https://groceristar.github.io/groceristar-fetch/
GNU General Public License v3.0
8 stars 18 forks source link

Intro to the project + rename helper.js to utils.js #336

Open atherdon opened 5 years ago

atherdon commented 5 years ago

You need to read readme file: https://github.com/GroceriStar/groceristar-fetch/blob/master/README.md it also contains links to articles, where I explaining some details. If something will be not clear - don't hesitate and ask questions.

Tasks, that you can take(feel free to ask clarification if something is not clear)

  1. rename helper.js to utils.js note it's not that easy as it can look like. take time and pay attention.

@Beni03

Beni03 commented 5 years ago

I changed the file name. Is it what you expected?

atherdon commented 5 years ago

nope, i send you my comments

Beni03 commented 5 years ago

updated all the modules( under src/project) which uses helper.js file

atherdon commented 5 years ago

did you push that changes via pull request?

atherdon commented 5 years ago

i don't see your changes here: https://github.com/GroceriStar/groceristar-fetch/pull/339

Beni03 commented 5 years ago

Sorry! Yeah now I pushed all the changes via pull request

On Sat, Apr 13, 2019 at 6:52 AM Arthur Tkachenko notifications@github.com wrote:

i don't see your changes here: #339 https://github.com/GroceriStar/groceristar-fetch/pull/339

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GroceriStar/groceristar-fetch/issues/336#issuecomment-482811110, or mute the thread https://github.com/notifications/unsubscribe-auth/APgcs31_a8A9wsphCtu5NIQMRzNmEEDjks5vgeEAgaJpZM4ckMHN .

atherdon commented 5 years ago

no problems. awesome progress, btw Ok, so looks like this task is done, can you confirm that it works well and we can move forward? Btw, i saw that you saw few of my @todos - you update them. Maybe you can deal with that simple tasks too?

Beni03 commented 5 years ago

Yes, we can move forward.

Thanks, Benitha

On Sun, Apr 14, 2019 at 2:31 AM Arthur Tkachenko notifications@github.com wrote:

no problems. awesome progress, btw Ok, so looks like this task is done, can you confirm that it works well and we can move forward? Btw, i saw that you saw few of my @todos https://github.com/todos - you update them. Maybe you can deal with that simple tasks too?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GroceriStar/groceristar-fetch/issues/336#issuecomment-482937943, or mute the thread https://github.com/notifications/unsubscribe-auth/APgcsx_QIHdj2L7IukwCwWzWPrYfxnA8ks5vgvWCgaJpZM4ckMHN .

atherdon commented 5 years ago

cool. then please deal with todos that you've changed

atherdon commented 5 years ago

here when i saw it: https://github.com/GroceriStar/groceristar-fetch/pull/345/commits/44d1f30059465ecbc061f304a42deb1e7c64502b

Beni03 commented 5 years ago

Sure. Will work on it.

Sent from my iPhone

On Apr 15, 2019, at 4:22 AM, Arthur Tkachenko notifications@github.com wrote:

here when i saw it: 44d1f30

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Beni03 commented 5 years ago

Can I use crypto module to generate random IDs?

On Mon, Apr 15, 2019 at 10:12 AM ebi.benny@gmail.com wrote:

Sure. Will work on it.

Sent from my iPhone

On Apr 15, 2019, at 4:22 AM, Arthur Tkachenko notifications@github.com wrote:

here when i saw it: 44d1f30 https://github.com/GroceriStar/groceristar-fetch/commit/44d1f30059465ecbc061f304a42deb1e7c64502b

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GroceriStar/groceristar-fetch/issues/336#issuecomment-483212486, or mute the thread https://github.com/notifications/unsubscribe-auth/APgcs8h19Tyl6ubrqieKJhneiemPr4Peks5vhGD8gaJpZM4ckMHN .

Beni03 commented 5 years ago

Please ignore the above question.

On Thu, Apr 18, 2019 at 12:32 PM Benitha .D ebi.benny@gmail.com wrote:

Can I use crypto module to generate random IDs?

On Mon, Apr 15, 2019 at 10:12 AM ebi.benny@gmail.com wrote:

Sure. Will work on it.

Sent from my iPhone

On Apr 15, 2019, at 4:22 AM, Arthur Tkachenko notifications@github.com wrote:

here when i saw it: 44d1f30 https://github.com/GroceriStar/groceristar-fetch/commit/44d1f30059465ecbc061f304a42deb1e7c64502b

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GroceriStar/groceristar-fetch/issues/336#issuecomment-483212486, or mute the thread https://github.com/notifications/unsubscribe-auth/APgcs8h19Tyl6ubrqieKJhneiemPr4Peks5vhGD8gaJpZM4ckMHN .

Beni03 commented 5 years ago

const utils = require('./utils');

const getFiveRandomId = function () { return [ utils. generateId(), utils.generateId(), utils.generateId(), utils.generateId(), utils.__generateId() ] }

var result = getFiveRandomId(); console.log(result);

is this what you expected? looks like it is done already. On Thu, Apr 18, 2019 at 4:02 PM Benitha .D ebi.benny@gmail.com wrote:

Please ignore the above question.

On Thu, Apr 18, 2019 at 12:32 PM Benitha .D ebi.benny@gmail.com wrote:

Can I use crypto module to generate random IDs?

On Mon, Apr 15, 2019 at 10:12 AM ebi.benny@gmail.com wrote:

Sure. Will work on it.

Sent from my iPhone

On Apr 15, 2019, at 4:22 AM, Arthur Tkachenko notifications@github.com wrote:

here when i saw it: 44d1f30 https://github.com/GroceriStar/groceristar-fetch/commit/44d1f30059465ecbc061f304a42deb1e7c64502b

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GroceriStar/groceristar-fetch/issues/336#issuecomment-483212486, or mute the thread https://github.com/notifications/unsubscribe-auth/APgcs8h19Tyl6ubrqieKJhneiemPr4Peks5vhGD8gaJpZM4ckMHN .

atherdon commented 5 years ago

maybe it works, but i still think that this current approach is not ideal. i think by combining some lodash method with generateId we can make it better

Beni03 commented 5 years ago

I have added the lodash method to generate unique ID

On Fri, Apr 19, 2019 at 2:18 AM Arthur Tkachenko notifications@github.com wrote:

maybe it works, but i still think that this current approach is not ideal. i think by combining some lodash method with generateId we can make it better

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GroceriStar/groceristar-fetch/issues/336#issuecomment-484823159, or mute the thread https://github.com/notifications/unsubscribe-auth/AD4BZM7LNCM4CHAC7F74BZTPRGE5LANCNFSM4HEQYHGQ .

atherdon commented 5 years ago

yeah, take alook at this comment: https://github.com/GroceriStar/groceristar-fetch/pull/354#issuecomment-485362634

Beni03 commented 5 years ago

Yes, it’s possible to generate Id using lodash. But, I think the generated Id’s will look like auto increment key values. Is that what we want?

Sent from my iPhone

On Apr 22, 2019, at 3:11 AM, Arthur Tkachenko notifications@github.com wrote:

yeah, take alook at this comment: #354 (comment)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

atherdon commented 5 years ago

@vadim9999 please jump here too.

vadim9999 commented 5 years ago

I think generate Id using lodash is for simple task and easy to check what is relationship beetwen objects. uuidv is for more complex task. I think it more secure. Hacker should know this randomly generated id with characters and numbers. Also max value of number is limited but what he will do when max limit will be reach? In simple DB like Hello Wrld I used just increment numbers but it should be unique. Key is like a cipher to identify user. Here we are using objects