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

fix: undo invalid refactoring #124

Merged
merged 1 commit into from
May 10, 2024
Merged

fix: undo invalid refactoring #124

merged 1 commit into from
May 10, 2024

Conversation

Dorus
Copy link
Collaborator

@Dorus Dorus commented May 10, 2024

See #116 (comment)

I missed this one, but this is not a valid refactoring.

afbeelding

This gives a new error in the latest master: afbeelding

On a separate note: Export project page to cliborad exports json, import project page from clipboard expect base64. So i couldnt test this function, but as you see in the watch screenshot it's not working as intended.

Notice the refactor operator did warn this might not be a correct refactoring:
afbeelding

@Dorus
Copy link
Collaborator Author

Dorus commented May 10, 2024

While i'm at it, master also couldn't build, so i fixed this one too:

afbeelding

How did this get past the build check?

Edit: my local git was seeing ghosts.

@shpaass
Copy link
Owner

shpaass commented May 10, 2024

Thank you for the catch!
However, do I understand it correctly that your fix of the CreateSolver error was to revert most of the changes about it?

@Dorus
Copy link
Collaborator Author

Dorus commented May 10, 2024

Wait, something is messed up there, let me try to fix it. It should be a 1 line change...

@Dorus
Copy link
Collaborator Author

Dorus commented May 10, 2024

While i'm at it, master also couldn't build, so i fixed this one too:

afbeelding

How did this get past the build check?

To answer my own question: this was an issue with my local git, not with the master branch. Idk why but somehow this file didnt update when i pulled it. Actually i have a suspicion that i had it open locally and oversaved it with an old version. All reverted now.

@shpaass
Copy link
Owner

shpaass commented May 10, 2024

Regarding the error with pattern matching, the problem was that many applications of pattern matching have (may change semantics) when applying them. Therefore, I did not notice (may change code meaning).

@Dorus
Copy link
Collaborator Author

Dorus commented May 10, 2024

I double checked all other pattern patching changes, and the rest of them are sound.

@shpaass shpaass merged commit 2cf2c0b into shpaass:master May 10, 2024
2 checks passed
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