-
Notifications
You must be signed in to change notification settings - Fork 117
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
Setup sourcegraph for scala 2 #379
Setup sourcegraph for scala 2 #379
Conversation
859073b
to
ee53085
Compare
build.sbt
Outdated
List( | ||
semanticdbEnabled := true, | ||
semanticdbVersion := "4.4.33" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would like to enable that ad-hoc during the CI build, but it seems that right know there is no way to do that via lsif-java interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does enabling semantic db have any side-effects?
also, why can't we enable this only for the duration of the build? how to w egenerate the dump.lsif
file in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does enabling semantic db have any side-effects?
Yes, it might increase the build time.
Also, I imagine that something might break some time inside of semanticDb which will cause the compilation to fail.
also, why can't we enable this only for the duration of the build? how to w egenerate the dump.lsif file in the first place?
We could use: https://github.com/sourcegraph/sbt-sourcegraph#other-ways-to-enable-semanticdb
but the problem is that we are not calling sbt-sourcegraph ourselves rather it is managed by lsif-java.
One way would be to get rid of lsif-java and manually call sbt-sourcegraph but I would like to avoid that as lsif-java adds dynamically sbt-sourcegraph plugin to the project.
The other option would be to attach that information during the build using text file manipulation which would work but it's quite hacky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so lsif-java runs some sbt command? can't we scope the settings so that they are used only when this command is run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you scope the settings to be used for a command which does not exist? Remember that java-lsif adds plugin to sbt during execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm no you can't ... I guess I'd prefer to add the plugin explicitly then. I don't want to slow compilation down (esp in sttp/tapir) or to introduce a new potential build-breaking feature, as the builds are already failing too often ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that makes sens. I will update the PR.
.github/workflows/main.yml
Outdated
with: | ||
endpoint: https://sourcegraph.com | ||
github_token: ${{ secrets.GITHUB_TOKEN }} | ||
file: dump.lsif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we need to tag the upload with the version or sth like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean magnolia version or scala version?
- magnolia version - The lsif upload will be associated with given commit hash, so no.
- scala version - Sourcegraph for now doesn't support cross-compiled projects. However, magnolia is kind of special as different branches build different versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, I meant the magnolia version for which we are sending the source info :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or is that in the dump file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not in the dump file as far as i know. The lsif upload will be associated with given commit hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lsif upload will be associated with given commit hash.
This is at least what I would expect it to do, but it is only assumption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct, you can check it at https://sourcegraph.com/github.com/softwaremill/magnolia/-/code-intelligence/uploads?visible=1
[github.com/softwaremill/magnolia](https://sourcegraph.com/github.com/softwaremill/magnolia)
Directory [/](https://sourcegraph.com/github.com/softwaremill/magnolia@f8c53577718c717b76fb625d8f47d69fb71a7f3f)
indexed at commit [f8c5357](https://sourcegraph.com/github.com/softwaremill/magnolia/-/commit/f8c53577718c717b76fb625d8f47d69fb71a7f3f) by lsif-java
Completed 1 minute ago
Merged :) Should we add a badge to readme.md now or sth like that? |
AFAIK for now there is a badge only for GO projects, so we have to leave with out it :( |
This PR apart from adding integration with sourcegraph also unifies how java is being set.
The unification part can be revert or done otherwise if desired.
Old
actions/setup-java@v1
supported only one distribution - Zulu OpenJDK (Note that v2 supports wider variety of distributions)That means PR also changes openjdk distro which is used by the CI pipeline.
This is a complementary PR for #369 which is currently on hold as something doesn't work correctly for scala3.