Skip to content

perf: rm railToDataSet mapping for cdn rails #278

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
Merged
merged 1 commit into from
Oct 10, 2025
Merged

perf: rm railToDataSet mapping for cdn rails #278

merged 1 commit into from
Oct 10, 2025

Conversation

@wjmelements
Copy link
Contributor

@wjmelements wjmelements commented Oct 10, 2025 •
edited
Loading

Reviewer @rvagg
CC @Chaitu-Tatipamula
Closes #277
Resolves #269 (review)
We are able to remove this mapping because it is only used by IValidator functions, and, since #237, FWSS is not the IValidator for these rails.

This reduces codesize 23,991 -> 23,898 (-93)

Changes

  • remove railToDataSet mapping for both cdn rails

@wjmelements wjmelements requested a review from rvagg October 10, 2025 05:56
@FilOzzy FilOzzy added this to FS Oct 10, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Oct 10, 2025
@wjmelements wjmelements mentioned this pull request Oct 10, 2025
Merged
rvagg
rvagg approved these changes Oct 10, 2025
View reviewed changes
Copy link
Collaborator

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

I'm not seeing in #237 where we stop needing to touch this, was that use removed before that PR? I know we used them for a little while in the SDK but that got fixed a little while back (I still see it in the mocks though)

@github-project-automation github-project-automation bot moved this from 📌 Triage to ✔️ Approved by reviewer in FS Oct 10, 2025
@wjmelements
Copy link
Contributor Author

wjmelements commented Oct 10, 2025

I'm not seeing in #237 where we stop needing to touch this, was that use removed before that PR? I know we used them for a little while in the SDK but that got fixed a little while back (I still see it in the mocks though)

railToDataSet is used by the IValidator callbacks, but #237 changes the payment calculation so that the CDN rails do not use FWSS for IValidator.

@wjmelements wjmelements merged commit fab1413 into main Oct 10, 2025
6 checks passed
@wjmelements wjmelements deleted the rm-cdn-rail-to-dataset branch October 10, 2025 06:40
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FS Oct 10, 2025
@rjan90 rjan90 mentioned this pull request Oct 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rvagg rvagg rvagg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

FS
Status: 🎉 Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

perf: Do not need to set railToDataSet for cdn rails

3 participants

@wjmelements @rvagg