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

Vals and vars #483

Merged
merged 3 commits into from
Oct 22, 2023
Merged

Vals and vars #483

merged 3 commits into from
Oct 22, 2023

Conversation

FWDekker
Copy link
Owner

  1. Ensures consistent usage of val/var in Schemes (i.e. fixes Check usage of var vs val in all schemes #476).
  2. This required some changes in the Tree classes, which ended up being a major overhaul of how they work together.

Notes from one of the commits:

I found quite a few pieces of code that were, by themselves, neat, but for which much better solutions existed. With these changes, I feel more confident in the stability of the code, and plenty of docs and tests have been added as well. I am aware that #473 will be another overhaul of the Trees, but that won't be until v3.1.x, and I don't want to wait for that. Also, depending on the solution for that issue, the code here might be retained completely anyway.

The goal was actually only to fix the usages of `val`s and `var`s across the `Scheme`s (cf. #476), but this ended up in being a major overhaul of how the various `Tree`-related classes work together. I found quite a few pieces of code that were, by themselves, neat, but for which much better solutions existed. With these changes, I feel more confident in the stability of the code, and plenty of docs and tests have been added as well. I am aware that #473 will be another overhaul of the `Tree`s, but that won't be until v3.1.x, and I don't want to wait for that. Also, depending on the solution for that issue, the code here might be retained completely anyway.
@FWDekker FWDekker added the code quality Code changes without behavior changes label Oct 19, 2023
@FWDekker FWDekker added this to the v3.0.0 milestone Oct 19, 2023
@FWDekker FWDekker self-assigned this Oct 19, 2023
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #483 (0af6ce3) into main (823f872) will decrease coverage by 0.13%.
The diff coverage is 91.41%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   84.93%   84.81%   -0.13%     
==========================================
  Files          51       51              
  Lines        1899     1896       -3     
  Branches      328      319       -9     
==========================================
- Hits         1613     1608       -5     
- Misses        225      229       +4     
+ Partials       61       59       -2     

@FWDekker FWDekker merged commit 3375990 into main Oct 22, 2023
4 checks passed
@FWDekker FWDekker deleted the vals-and-vars branch October 22, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Code changes without behavior changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check usage of var vs val in all schemes
1 participant