Skip to content

Update config.json #834

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

Merged
merged 3 commits into from
Oct 3, 2017
Merged

Update config.json #834

merged 3 commits into from
Oct 3, 2017

Conversation

Akatsuki06
Copy link
Contributor

added topics for the queen-attack problem
fixes #829


Reviewer Resources:

Track Policies

added topics for the queen-attack problem
Copy link
Contributor

@FridaTveit FridaTveit left a comment

Choose a reason for hiding this comment

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

This looks great @Akatsuki06! :) I've suggested one more topic but apart from that it's ready to merge! :)

Also just a small suggestion for the future, it can be helpful if the title of the pull request very clearly states what the pull request is for. For example, for this pull request "queen-attack: add topics" or something like that would make it very obvious what it's about. You state it very nicely in the description but I can't see that when looking at the files, and since I'm doing very many reviews at the moment I can easily forget which review I'm doing :P

But that's only a very minor thing and apart from that this PR looks great! :)

config.json Outdated
"arrays",
"matrices",
"conditionals",
"mathematics"
Copy link
Contributor

Choose a reason for hiding this comment

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

These look like great suggestions! :) I would also add games as a topic since this is to do with chess :) Do you agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FridaTveit
games and also how about tuples ? Can we add even though its not java datatype? or I should specify as classes as this specific program contains custom class BoardCoordinate .

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't add tuples since it's not a java datatype so it could be confusing. But classes is a very good suggestion! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have done the updates

Copy link
Contributor

@FridaTveit FridaTveit left a comment

Choose a reason for hiding this comment

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

This looks great @Akatsuki06! :) Will merge it as soon as the build finishes :)

Thanks for taking the time to do the changes! We really appreciate the help :)

@FridaTveit FridaTveit merged commit 922d07b into exercism:master Oct 3, 2017
@Akatsuki06 Akatsuki06 deleted the patch-1 branch October 3, 2017 16:08
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.

queen-attack: add topics
2 participants