-
Notifications
You must be signed in to change notification settings - Fork 28
Suffix module names with 'Fork' to avoid conflicts with newer versions of the same modules included in CommunityModules #239
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
Conversation
78e996e to
9114f18
Compare
…s of the same modules included in CommunityModules. Workaround for Github issue #171 #171 [Bug] Signed-off-by: Markus Alexander Kuppe <[email protected]>
|
I don't think this is a good approach and seems downstream of the general lack of a well-thought-out versioning or packaging system for the community modules. Perhaps these should be moved out of the TLAPM standard library and into the community modules themselves? |
|
Right now, the main issue is that the outdated |
|
I think moving the modules out of TLAPM into the community modules sounds good. |
|
This is blocked because the proofs fail for the latest versions of |
|
Perhaps they can be commented out then turned into an |
|
This PR does not block any of that work or make fixing the proofs more difficult. The specific names of these two models should not matter. If anything, the *Fork prefix more clearly indicates that these modules are forks (of the ones in CM). Most importantly, it ensures that VSCode users are not broken. |
|
It will break the proofs of anybody who uses these modules. As would moving these modules to the community modules, but then users are presented the upgrade path of moving to a new version of the community modules instead of renaming their imports temporarily. Since the goal state has these modules named the same as they are now, switching their names back and forth adds an unnecessary migration step. |
|
Good point — those users (if they exist) would also be affected by #171 |
|
See comment about upgrade paths. Fundamentally I see this as a shortcut that makes the situation worse when a bit more work (moving the modules to the community repo and commenting out proofs that no longer currently work) would lead to a better outcome. |
Are you volunteering? I’m afraid I won’t be able to do more than this PR. |
|
By the way, even if Functions and SequencesExt were moved to CommunityModules, existing TLAPS proofs would still break until authors explicitly include CommunityModules. |
tlaplus/tlapm#239 Signed-off-by: Markus Alexander Kuppe <[email protected]>
tlaplus/tlapm#239 Signed-off-by: Markus Alexander Kuppe <[email protected]>
tlaplus/tlapm#239 Signed-off-by: Markus Alexander Kuppe <[email protected]>
tlaplus/tlapm#239 Signed-off-by: Markus Alexander Kuppe <[email protected]>
tlaplus/tlapm#239 Signed-off-by: Markus Alexander Kuppe <[email protected]>
Workaround for Github issue #171
#171
[Bug]