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

Revert "Add RSA support on encodeString (#2412)" #2472

Conversation

TheNormalnij
Copy link
Contributor

This reverts commit e7e3ba5.

Causes regression on client when compiled in debug mode. Didn't notice it this when i was on release.
@Inder00, recheck tests, please.

@Dutchman101
Copy link
Member

Dutchman101 commented Dec 22, 2021

Causes regression on client when compiled in debug mode

Can you describe it here?

Btw, my personal opinion is that reverting it is low priority and to be delayed as long as possible, in favour of the fixing alternative. Because first of all, it working on release builds is the priority (In the meanwhile, any developer that builds MTA and wants to try using encodeString with the new feature, can revert it locally after taking note) so the chance of a niche case where a developer will have a bug on custom builds, still provides you enough time and room to fix the issue instead of choosing to revert.

I'm pulling on this a bit because we're trying to get the perfect stable build (a few changes and PR's are in line for it), and RSA support would be a nice addition for a widely circulated build soon.. keeping it in master for now serves to get to that point. So as long it really just affects Debug builds, this applies. And yeah, feel free to describe the regression so we can determine if that's expected to be the case.

@TheNormalnij
Copy link
Contributor Author

So we can just disable broken test

@Dutchman101
Copy link
Member

So we can just disable broken test

Yes, that sounds better.. still, do you have any details? The tests aren't important for release MTA builds (after the feature got implemented already) right?

@Dutchman101
Copy link
Member

Replaced by PR #2475

@TheNormalnij TheNormalnij deleted the TheNormalnij/rsa_revert branch December 24, 2021 16:49
@patrikjuvonen patrikjuvonen added the bug Something isn't working label Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants