Remove the pinned version on solargraph dependency #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
By marking items as
I also notice that the rails7 routing specs point to rails6/routes.yml ; fix in #50 |
|
@iftheshoefritz that's why I was so reluctant to release version requirements, I wasn't getting any consistent results. The code relies heavily on solargraph internals, and we cannot just yolo bump. To be more confident we need to spend more time in testing infra, which me personally don't have right now :( |
iftheshoefritz
force-pushed
the
unversioned-solargraph
branch
from
7ce929e
to
1211f63
Compare
iftheshoefritz
force-pushed
the
unversioned-solargraph
branch
3 times, most recently
from
200f048
to
2155152
Compare
This is a bit of a hack, I don't know why solargraph wouldn't include .descendants and/or whether it actually is included in any version of Rails. But if this one change is enough to keep the tests passing I don't really care to investigate deeply. If other stuff fails, then we probably have a deeper problem.
iftheshoefritz
force-pushed
the
unversioned-solargraph
branch
from
2155152
to
8d70396
Compare
Testing multiple definitions in a single spec means that if one file has a problem, the entire test fails and we get no feedback about what might else have failed. Now, move the loop outside the test so that each definition file has its own test and we can get feedback on all files at once.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We get constant PRs from people trying to update the version of solargraph because dependant is complaining. I'm not sure that we benefit from pinning it anymore. I think that the versions that have real problems for solargraph-rails are quite old now.
Although there is a PR for this already, we're waiting for it to be updated with the latest commits on main, I'd rather get this in asap.