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

Remove NameError rescue in page base #1350

Closed
wants to merge 1 commit into from
Closed

Remove NameError rescue in page base #1350

wants to merge 1 commit into from

Conversation

c4lliope
Copy link
Contributor

@c4lliope c4lliope commented May 20, 2019 •
edited

Closes #920;
Fixes #480.

This pull request overrides #920 in an effort to keep the ball moving.

@c4lliope c4lliope added the Merge label May 20, 2019
@GeoffAbtion
Copy link

  • How are the timestamps related to the NameError rescue?

  • Should there be tests?

@nickcharlton nickcharlton removed the Merge label Jan 2, 2020
@pablobm
Copy link
Collaborator

pablobm commented Feb 7, 2020

Duplicate of #920. That PR explains the (very relevant) questions about timestamps:

The rescue statement generates a false positive on payments feature specs, the timestamps migrations are created to fix this.

I think that also sorts the need for tests, fixing that false positive that is raised in the existing ones.

@pablobm pablobm closed this Feb 7, 2020
@nickcharlton nickcharlton deleted the abandon-nameerrors branch October 5, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NoMethodError is silently ignored
5 participants
@c4lliope @GeoffAbtion @pablobm @nickcharlton @ndrluis