Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug #0000099 fix #1

Merged
merged 2 commits into from
Apr 7, 2014
Merged

Bug #0000099 fix #1

merged 2 commits into from
Apr 7, 2014

Conversation

slovasim
Copy link
Contributor

Empty name input fixed.
New table creation fixed.

Empty name input fixed.
New table creation fixed.
@b0n541
Copy link
Owner

b0n541 commented Mar 31, 2014

Thanks for the pull request. Would you please correct two minor things, before I merge the changes:

1.) Method JSkatMaster.createTable()

After checking whether the table name is empty and createTable() is called again a return has to be inserted directly after the call to createTable(). Otherwise you get two tables created after you try to put in an empty name and then putting in a correct name on the second try.

2.) No need to put JSkatMaster as parameter of constructor of JSkatTabComponent

JSkatMaster is a singleton and you can call JSkatMaster.instance() everywhere you need to call a method of JSkatMaster. The same could be done for JSkatGraphicRepository, but this would be too much for the scope of this bugfix.

I hope this makes sense and you can change these things easily. Just try it. Otherwise I can do these changes after the merge. Thanks again.

@slovasim
Copy link
Contributor Author

Hi,

thank you for your patience and help. I will look at it on thursday or
at the weekend.

Best regards
Simon

@slovasim
Copy link
Contributor Author

slovasim commented Apr 3, 2014

Hi,

I have just finished it. I don't need to do another pull request, do I?

@b0n541
Copy link
Owner

b0n541 commented Apr 3, 2014

Cool! No you don't have to do another pull request. I'll look at it on the weekend.

Do you want your name included in the contributor list on the splash screen? If yes, please send me your real name. Otherwise I can take your user name 'slovasim'.

@slovasim
Copy link
Contributor Author

slovasim commented Apr 3, 2014

slovasim is OK

Thank you very much

@b0n541 b0n541 merged commit 491ff16 into b0n541:master Apr 7, 2014
@b0n541
Copy link
Owner

b0n541 commented Apr 7, 2014

I merged your bugfix successfully. I hope this is enough to succeed for your task at the university. If not, please write me again.

If you are interested to contribute more to JSkat, feel free to ask me for some tasks...

Thanks again and happy coding.

@slovasim
Copy link
Contributor Author

Yes, it is enough. Thank you very much. You can help me more if you have project documentation, but a supose that you don't have it, do you?

@b0n541
Copy link
Owner

b0n541 commented Apr 16, 2014

Unfortunately there is no written documentation available apart from the generated Javadoc. Sorry.

@slovasim
Copy link
Contributor Author

It is ok, doesn't matter.

Thanks and have a nice day
Simon

@b0n541
Copy link
Owner

b0n541 commented Apr 18, 2014

I found a little documentation done be another student who forked JSkat a while ago:

https://github.com/patrickayoup/jskat-multimodule/blob/master/JSkatMilestone3.pdf

Maybe it is of a little help...

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.

None yet

2 participants