Skip to content
This repository was archived by the owner on Jan 17, 2019. It is now read-only.

Conversation

NCCastillo
Copy link
Contributor

This should fix issue #39

Summary:

  • created new TestFile object which will handle the logic to determine if there is a test pattern in the config. If
    not then use /test/i as the default
  • expanded the functionality of Problem to have a method that returns the test file directly from the new TestFile object
  • added new end point problems/:track/:slug/tests

Note:
There were some failures with some of the approvals. I verified and approved them to get them to pass since the differences were in the "README.md" part of the file.

Copy link
Member

Choose a reason for hiding this comment

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

We don't need the deprecated node since this is a new file. I've only kept the deprecated one around where the CLI was previously using the result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha

  • TODO: remove deprecated

@kytrinyx
Copy link
Member

kytrinyx commented Mar 3, 2015

This looks great, thank you, there's just that one thing about the deprecated node, other than that I think this is ready.

I'm not exactly sure how the submodule updates ended up in this PR, and I'd like to figure out how to remove those changes since they don't have anything to do with the problem test endpoint.

Would you try rebasing onto master?

Also, once everything is ready, would you squash the commits?

@NCCastillo
Copy link
Contributor Author

I will work on these changes in the next few days. Thanks for the feedback.

  • TODO: Squash commits when everything is ready.

@kytrinyx
Copy link
Member

kytrinyx commented Mar 4, 2015

OMG I love the use of the checklist/TODO in the comments. I am totally stealing that.

@NCCastillo NCCastillo force-pushed the add-endpoint-tests branch from 92bc0a8 to cc02ed6 Compare March 4, 2015 18:26
@NCCastillo NCCastillo force-pushed the add-endpoint-tests branch from 4b2c64c to d10d1ab Compare March 4, 2015 23:59
@NCCastillo
Copy link
Contributor Author

Steal away - thats the only way I will remember to do it 😀

I did all the changes as requested (removing the submodule commits and squashing the rest of commits) however I just noticed Travis has 3 failures with some of the approvals. All tests pass locally for me so I am not sure what to do next. Any suggestions?

p.s. I could of derped something when I was rebasing/squashing

@kytrinyx
Copy link
Member

kytrinyx commented Mar 5, 2015

Weird, I'll pull it down and see what I can figure out.

@kytrinyx
Copy link
Member

kytrinyx commented Mar 5, 2015

Ah, sheesh. Going through this I realised that I pushed a "wip" commit to master sometime in the past few days, and I have no recollection of doing so! sigh I do wish my brain would work better sometimes.

@kytrinyx
Copy link
Member

kytrinyx commented Mar 5, 2015

Yepp, they were failing locally for me. I simply accepted them and amended your commit and merged it locally. It's all good ✨ :)

Deploying now. Thanks!

@kytrinyx kytrinyx closed this Mar 5, 2015
@NCCastillo
Copy link
Contributor Author

👍 thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants