Skip to content

Fix params #56

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

Closed
wants to merge 4 commits into from
Closed

Conversation

afrind
Copy link
Contributor

@afrind afrind commented May 11, 2025

Summary:
There were several problems here:

v11 integer params have no length
includeParam was wrong for setup

Reviewed By: sharmafb

Differential Revision: D74500602

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 11, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74500602

@afrind afrind force-pushed the export-D74500602 branch from b038231 to 21915c8 Compare May 12, 2025 21:26
afrind added a commit to afrind/moxygen that referenced this pull request May 12, 2025
Summary:

There were several problems here:

v11 integer params have no length
includeParam was wrong for setup

Reviewed By: sharmafb

Differential Revision: D74500602
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74500602

afrind added a commit to afrind/moxygen that referenced this pull request May 12, 2025
Summary:
Pull Request resolved: facebookexperimental#56

There were several problems here:

v11 integer params have no length
includeParam was wrong for setup

Reviewed By: sharmafb

Differential Revision: D74500602
@afrind afrind force-pushed the export-D74500602 branch from 21915c8 to 2601360 Compare May 12, 2025 21:31
afrind added 3 commits May 13, 2025 09:29
Summary: There's a marginally gross hack here where I keep a single map of "FullTrackName" to RequestID for legacy verions.  For TrackStatus its the real FTN, for ANNOUNCE and SUB_ANNOUNCES it's a fake FTN with eg (NS, "announce").  It could break if someone has a track name or namespace tuple called "announce" or "subannounce".

Differential Revision: https://www.internalfb.com/diff/D74147358
Differential Revision: D74530554
Differential Revision: D74500562
afrind added a commit to afrind/moxygen that referenced this pull request May 13, 2025
Summary:

There were several problems here:

v11 integer params have no length
includeParam was wrong for setup

Reviewed By: sharmafb

Differential Revision: D74500602
@afrind afrind force-pushed the export-D74500602 branch from 2601360 to b234494 Compare May 13, 2025 17:14
Summary:
Pull Request resolved: facebookexperimental#56

There were several problems here:

v11 integer params have no length
includeParam was wrong for setup

Reviewed By: sharmafb

Differential Revision: D74500602
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74500602

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 6a0d512.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants