dionyziz / Automata

An editor for deterministic finite automata
http://automata.discrete.gr
51 stars 12 forks source link

Deploy #42

Closed vasspilka closed 9 years ago

vasspilka commented 9 years ago

Backend has been rewritten, keeping only a small amount of previous code. Addition of of js functions in server.js for handling users. Some code in ui.js to render name and image of user.

The backend can now save automatons both under a user or without one , supports sessions through google login, and has the api nessessary to handle basic user interactions for automata.

Things missing are: The backend for deleting and modifying automata. The UI for user to handle his automata.

DmitryMyadzelets commented 9 years ago

What is the motivation behind renaming the "server" to "backend"? Citing Wikipedia, front-end and back-end "refer to the separation of concerns between a presentation layer and a data access layer respectively". In case of this tool most of work happens on the client's side, i.e. it's far from just a presentation layer.

DmitryMyadzelets commented 9 years ago

It looks like a lot of change is going to be made by this PR. Could you, please show it working on github.io?

vasspilka commented 9 years ago

You can see it running on http://automata.discrete.gr/beta

I am not sure why I renamed it to backend... I did not think about it much. I guess its because when I started working on the project I could not run it correctly so I rewritten it completely, later,I realised how things where actually working and "merged" my work with the old one then I just left it as it was...

I don't think its hard to change though....

dionyziz commented 9 years ago

Please also take care of #43

dionyziz commented 9 years ago

Please squash as needed to avoid commits which go back and edit mistakes within the same PR.

vasspilka commented 9 years ago

The code mentioned in #43 was fixed in https://github.com/vasspilka/Automata/commit/a0890b66a12b13d10a87f8a1f1b5aaf07d82bdc4 about 3 weeks ago

Please don't make me remove the misspelled word "automatons" from history it's quite some trouble and I'd like to move onto other things.

DmitryMyadzelets its great having someone reviewing my code, I honestly appreciate it! But please don't dig to details as history on something like this. Issues about the current code would be awsome!! :)

vasspilka commented 9 years ago

If there are no other issues with this PR, it would be great if we could merge. I have some other stuff I want to work on regarding Automata, and it would be nice to see my work accepted.

vasspilka commented 9 years ago

If you want me to rename backend to server, move the (js,css,images) folders from /site to / or anything like this. I can do it with no problem just tell me please.

DmitryMyadzelets commented 9 years ago

vasspilka, I was looking at your PR. You see, your PR is not squashed, that's why it's hard to review it. For the same reason you've been asked to do it by dionyziz.

What are you going to work on? Please, share. Dionyziz could assign particular tasks to you. I had some insights either.

vasspilka commented 9 years ago

You can see the whole PR as being squashed to one commit in Files Changed

I am building the application running on the server, so the Automata website can support users and automata storage. There was one previously but I felt it could use a simpler structure so I've rewriten most of it, plus there was no support for users or local development among other things. I also had to write some javascript and modify the DOM to support my functions.

I was and still am in contact with Dio, but he is a busy person you know.. :P Hopefully we will sort out this PR soon :) .

:EDIT: I talked with Dio and I now understand the confusion. I'll try to make the commits more comprehensive and accurate

dionyziz commented 9 years ago

Please add screenshots for the UI changes.

dionyziz commented 9 years ago

I agree it would be nice to see this running on github.io. Please also make sure you rebase this over the recently merged changes by @DmitryMyadzelets so that it merges cleanly to master. Thanks!

vasspilka commented 9 years ago

I don't think github.io supports python... I will rebase over the changes :) . Nice to see it running fine on firefox

dionyziz commented 9 years ago

OK sounds good, maybe you want to publish the link you showed me where we can see it running so that @DmitryMyadzelets can also check it?

vasspilka commented 9 years ago

Google login requires a valid url to redirect to. Plus okenos blocked hosting on ports except 80 (I think) I'd have to drop lamiatodo.gr and I don't really want to... I'd have to set up apache properly or learn better nginx to do it correctly...

dionyziz commented 9 years ago

We're moving production to linode today, so ignore okeanos restrictions.

On Sun, Dec 21, 2014 at 2:30 PM, Vasilis Spilka notifications@github.com wrote:

Google login requires a valid url to redirect to. Plus okenos blocked hosting on ports except 80 (I think) I'd have to drop lamiatodo.gr and I don't really want to... I'd have to set up apache properly or learn better nginx to do it correctly...

— Reply to this email directly or view it on GitHub https://github.com/dionyziz/Automata/pull/42#issuecomment-67770460.

vasspilka commented 9 years ago

Cool :) , I am doing the logging now + other changes you wanted me to. I hope to have everything ready in Tuesday, I am helping my parents with some stuff so I don't have the free time I'd like to.

vasspilka commented 9 years ago

selection_002 selection_001 selection_003

Screenshots

dionyziz commented 9 years ago

This looks good, please take care of the issues mentioned and let's merge it. I'm looking forward to deploying this. Thanks for including the screenshots.

vasspilka commented 9 years ago

Changed "Log in" To "Sign in" selection_004

vasspilka commented 9 years ago

It would be nice to test it on your new server before merging this. Just to be sure.

You could download deploy as a branch to your current deployment and run setup.

dionyziz commented 9 years ago

This pull request introduces a bug which can be reproduced as follows:

  1. Go to the home page.
  2. Click on share, copy/paste the URL into a new tab. You will see #v35 at the end.
  3. In the new page, click share and copy the URL.

Expected behavior: The URL should end in #v36

Actual behavior: The URL ends in #v35#v36

vasspilka commented 9 years ago

BUG

Expected behavior: The URL should end in #v36 Actual behavior: The URL ends in #v35#v36

Fixed

dionyziz commented 9 years ago

Please address the comments.