Skip to content
This repository has been archived by the owner on Nov 6, 2021. It is now read-only.

Add spec to ChildrenController #179

Merged
merged 1 commit into from
Aug 8, 2020
Merged

Add spec to ChildrenController #179

merged 1 commit into from
Aug 8, 2020

Conversation

EmersonManabuAraki
Copy link
Contributor

@EmersonManabuAraki EmersonManabuAraki commented Sep 6, 2019 •
edited
Loading

Resolves #151

Description

Add new spec to test ChildrenController

Type of change

  • Improvement

How Has This Been Tested?

Running specs

Screenshots

image

@EmersonManabuAraki EmersonManabuAraki mentioned this pull request Sep 6, 2019
Closed
@EmersonManabuAraki EmersonManabuAraki force-pushed the test-children-controller branch 2 times, most recently from 27ee78f to 7669f10 Compare September 6, 2019 17:27
@benreyn
Copy link
Member

benreyn commented Sep 12, 2019

Thanks a ton for this contribution @EmersonManabuAraki! Sorry I havent had a chance to review this yet, Im just getting re-oriented after a 2 week vacation. Ill make sure to give this a thorough review ASAP! 😄

@benreyn
Copy link
Member

benreyn commented Sep 16, 2019

Many of my comments from your other PR (#215) about let and before apply here as well. Again, these are all just personal preference, and I am happy to get this PR merged as is if you disagree or dont have the time to make the changes I mentioned.

Again, I REALLY appreciate your work on this!

@benreyn
Copy link
Member

benreyn commented Sep 20, 2019

@EmersonManabuAraki, I think the only blocker here is that we probably shouldnt be adding more controller specs when the prevailing wisdom is to start migrating controller specs to request specs. See #173 , #180 , #172 , #174 for some context, and please let me know if I can be helpful.

@benreyn benreyn mentioned this pull request Oct 20, 2019
Closed
@benreyn
Copy link
Member

benreyn commented Oct 20, 2019

@EmersonManabuAraki, are you still actively working on this? Can I be helpful in getting this across the finish line?

@EmersonManabuAraki
Copy link
Contributor Author

I'm not working on it anymore, you can across the finish line

@benreyn benreyn added 🙋 Help Wanted Extra attention is needed and removed waiting-reponse labels Oct 21, 2019
@benreyn benreyn self-assigned this Oct 24, 2019
@sarslanoglu
Copy link
Contributor

@benreyn What needs to be done in here? I can help

@benreyn
Copy link
Member

benreyn commented Feb 12, 2020

Basically we dont want to keep using controller specs, and want to use request specs moving forward.

@edwinthinks
Copy link
Collaborator

@EmersonManabuAraki sorry for the delayed response. I'll merge it in and move it over to request to the request spec. LGTM

@edwinthinks edwinthinks merged commit 516596a into rubyforgood:master Aug 8, 2020
@edwinthinks edwinthinks mentioned this pull request Aug 8, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@benreyn benreyn benreyn approved these changes

Assignees

@benreyn benreyn

Labels
🙋 Help Wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test Coverage: Children Controller
5 participants
@EmersonManabuAraki @benreyn @sarslanoglu @edwinthinks @EmersonAraki