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.

Sign up for GitHub

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

Jump to bottom

Implement D&D Character Exercise #676

Merged
merged 9 commits into from
May 21, 2019
Merged

Implement D&D Character Exercise #676

merged 9 commits into from
May 21, 2019

Conversation

Tyresius92
Copy link
Contributor

Implemented the D&D Character exercise

@Tyresius92
Copy link
Contributor Author

Hmm. Looks like I need to update config.json, and also run Make. I'll switch over to my Linux computer and fix this up. I believe I'll be able to do that this evening.

@tejasbubane tejasbubane mentioned this pull request May 9, 2019
Closed
@Tyresius92
Copy link
Contributor Author

Hey guys. I just ran make test and aside from running the tests, the changes it seems to have made is all in the package.json files. Is this the expected behavior?

Additionally, I can take a guess at most of the values in config.json, but how do I get the uuid?

@tejasbubane
Copy link
Member

Hey guys. I just ran make test and aside from running the tests, the changes it seems to have made is all in the package.json files. Is this the expected behavior?

@Tyresius92 Sorry for the confusion. make test copies package.json from root of this project to all exercise directories. This is to make sure that all package.json files are consistent (with same packages and versions) and we use the same in CI. When you create a new exercise, it is expected you copy the package.json from root of this project i.e. this file and don't make any changes. If changes are required there, please discuss since they will be propagated to all exercises. We have a CI script which ensures all exercise package.json are exactly the same.

Additionally, I can take a guess at most of the values in config.json, but how do I get the uuid?

Generate a new UUID by any tool of your choice.

@SleeplessByte
Copy link
Member

Generate a new UUID by any tool of your choice.

you can use configlet uuid if you don't want to use something else!

@Tyresius92
Copy link
Contributor Author

Hey guys, just wanted to make sure no further action is required from me on this PR?

SleeplessByte
SleeplessByte reviewed May 20, 2019
View reviewed changes
exercises/dnd-character/dnd-character.spec.js
test('ability scores only calculated once', () => {
const Drizzt = new Character();

expect(Drizzt.strength).toEqual(Drizzt.strength);
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we want to test all of them and not just strength?

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 thought about that, but canonical-data.json only has the check on strength. I can put in the others as well if you'd like though.

@SleeplessByte
Copy link
Member

@tejasbubane LGTM, what do you think?

tejasbubane
tejasbubane approved these changes May 21, 2019
View reviewed changes
Copy link
Member

@tejasbubane tejasbubane left a comment

Choose a reason for hiding this comment

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

Looks good. 🚀

Sorry about the newline noise with all package.json files.

@tejasbubane tejasbubane merged commit ee51e31 into exercism:master May 21, 2019
@Tyresius92 Tyresius92 mentioned this pull request Mar 30, 2021
Merged
4 tasks
This was referenced Jul 14, 2022
Merged
Open
@ErikSchierboom ErikSchierboom mentioned this pull request Jul 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SleeplessByte SleeplessByte SleeplessByte approved these changes

@tejasbubane tejasbubane tejasbubane approved these changes

Assignees
No one assigned
Labels
new exercise ✨ x:size/large Large amount of work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Tyresius92 @tejasbubane @SleeplessByte @ErikSchierboom