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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split spec common actions support helper #2653

Merged
merged 3 commits into from
Jun 12, 2018
Merged

Split spec common actions support helper #2653

merged 3 commits into from
Jun 12, 2018

Conversation

aamarill
Copy link

This PR is for issue #2644 posted by @voodoorai2000.

Notes

Please review the way I currently include the modules into the "CommonActions" module. I feel like there should be a cleaner way to include them.

Please review the current location of the relocated methods. On top of each method there is a comment with which files use that method. I hope that helps in figuring out where they should go.

Thanks! 馃檶

module Budgets
# spec/features/budgets/ballots_spec.rb
def expect_message_organizations_cannot_vote
#expect(page).to have_content 'Organisations are not permitted to vote.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this expectation is not being used, I think it'd be better to remove it altogether

visit new_officing_residence_path
end


Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, remove this trailing line

@aitbw
Copy link
Collaborator

aitbw commented May 28, 2018

Besides those small annotations, great job @aamarill ! 馃帀

@voodoorai2000
Copy link
Member

This is magnificent!
Thank you so much @aamarill 馃憦

The way to include the modules is great 馃憤
And the locations too 馃憣馃槍

Thank for the comments with the references to the code, they helped 馃槍
Feel free to clean them up
With that and @aitbw's comments I think we are ready to merge 馃憤

@aamarill
Copy link
Author

aamarill commented Jun 4, 2018

Sorry for the delay! Let me know if I missed anything 馃槃
Hope everyone has a great week!

@voodoorai2000
Copy link
Member

voodoorai2000 commented Jun 12, 2018

Awesome! Thanks @aamarill 馃帀

@voodoorai2000 voodoorai2000 merged commit 399cc5a into consuldemocracy:master Jun 12, 2018
@aamarill
Copy link
Author

It was truly a pleasure to work with you all 馃槃 馃檹 Thank you for your words of encouragement and support!

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

3 participants