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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove timestamp constraint #370

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

chao-xian
Copy link
Contributor

These were only made explicit to support a potential rollback to Grape as we migrated from there to Rails in the past. However this is now causing us issues as part of our Rails 7.1 upgrade 1

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

@chao-xian chao-xian requested a review from aliuk2012 November 15, 2023 14:10
@chao-xian chao-xian marked this pull request as ready for review November 15, 2023 14:10
These were only made explicit to support a potential rollback to Grape as we migrated from there to Rails in the past. However this is now causing us issues as part of our Rails 7.1 upgrade [1]

[1]: https://github.com/alphagov/forms-api/actions/runs/6876807358/job/18703161928?pr=369
@chao-xian chao-xian force-pushed the remove-grape-timestamp-regression branch from 0e3299c to 28b81d3 Compare November 15, 2023 15:16
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@chao-xian chao-xian added this pull request to the merge queue Nov 15, 2023
Merged via the queue into main with commit 04d2b95 Nov 15, 2023
6 checks passed
@chao-xian chao-xian deleted the remove-grape-timestamp-regression branch November 15, 2023 15:23
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.

2 participants