lucasmontano / magic-link

Generate, send and validate a magic link.
MIT License
106 stars 13 forks source link

adding Content for README.md #3

Closed ArthurFleischman closed 4 years ago

ArthurFleischman commented 4 years ago

now i think everything will be ok

lucasmontano commented 4 years ago

Thanks for your PR! But I’m not sure why you’re addressing framework implementation here, as we still not over with the issue #2

IMO: It seems like to much for one PR that was supposed to add the initial Readme

ArthurFleischman commented 4 years ago

Thanks for your PR! But I’m not sure why you’re addressing framework implementation here, as we still not over with the issue #2

IMO: It seems like to much for one PR that was supposed to add the initial Readme

Oh, ok. Ill slow down...

ArthurFleischman commented 4 years ago

now its on our parameters

fernandovmp commented 4 years ago

I think open a new PR will be better, since merge this will pollute the commit history with an unused flask implementation and with the venv directory (which should be added to gitignore)

ArthurFleischman commented 4 years ago

now its just README.md for my pull request, @HenriqueDerosa @lucasmontano.

ArthurFleischman commented 4 years ago

hope its what you want...

fernandovmp commented 4 years ago

When i say to open a new PR, i mean create a new branch for it. If you look at commits section you'll see the entire history where only the lasts commits are relevance, and merge all of then isn't the best option.

Other option is rebase all the branch, but this will be harder than create a new PR.

ArthurFleischman commented 4 years ago

@fernandovmp ill do it

ArthurFleischman commented 4 years ago

but, is the content ok?

fernandovmp commented 4 years ago

@ArthurFleischman to the readme? I belive yes, but i did't have expirience with python projects yet, so i don't know how is the ideal setup guide.

ArthurFleischman commented 4 years ago

@fernandovmp i just putted an extra layer, thats VENV looks like a dcoker container. so nobody needs to install dependencies... just the first person

fernandovmp commented 4 years ago

Looks good, however I found here that the venv directory should be ignored and a requiriments.txt created.

ArthurFleischman commented 4 years ago

@fernandovmp i saw it too, but is the default .gitignore, we have no reason to dont use it, with i command we start using the python interpreter inside the VENV and also all the libs, and with other, we return to normal. we also have the switch button in vscode to select venv. Screenshot from 2020-05-11 21-01-17

fernandovmp commented 4 years ago

Huumm... I don't know exactly about it, the venv/bin/activate have a path of your local machine, but if we switch this to an relative path this should not have to create any conflict, also I notice that the previous branch have a 2mb size and I think that the venv directory is the problem. However I haven't enough knowledge about the venv so I will leave the decision of use it in setup (without the gitignore) for you or any other with experience.

ArthurFleischman commented 4 years ago

@fernandovmp this 2mb of filles will need to be created in everybody setup, we are just providing a default one with everything on, so less setup + code... its my personal opinion.

ArthurFleischman commented 4 years ago

@fernandovmp move this conversation to the new pull request