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

[feature] manual tag support #236

Closed
wants to merge 39 commits into from
Closed

[feature] manual tag support #236

wants to merge 39 commits into from

Conversation

basakkapusuzoglu
Copy link
Contributor

@basakkapusuzoglu basakkapusuzoglu commented May 3, 2022

I added manual tag, when this tag is used build, run and test will not be done without explicitly asked.


https://youtrack.jetbrains.com/issue/BAZEL-40


testing:

manual testing

Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, probably u havent used the google java formatter, since there are 4 spaces tabs - please fix it.

Also please change the PR name and a description, update changelog.

PS did u manage to test it manually?

@basakkapusuzoglu
Copy link
Contributor Author

I tested it manually. I ./install.sh the file first and in bazelbsp.trace.json I ctrl+F and looked for the number of "canCompile": false, then
I added tags= ["manual"] to build file. Resynced. ./install.sh the file and when I ctrl+F in bazelbsp.trace.json I realized that the number of "canCompile": false, increased. So I thought that I did it correctly

@basakkapusuzoglu basakkapusuzoglu changed the title Issue 40 [addition] Manual tag added May 3, 2022
@abrams27 abrams27 changed the title [addition] Manual tag added [feature] Manual tag added May 3, 2022
@abrams27 abrams27 changed the title [feature] Manual tag added [feature] manual tag support May 3, 2022
Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can write some e2e tests as well

@@ -27,3 +27,10 @@ data class ProjectViewBazelPathSection(override val value: Path) :
const val SECTION_NAME = "bazel_path"
}
}

data class ProjectViewManualSection(override val value: Boolean) :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update the name as well ProjectViewBuildManualTargetsSection

val javaPath: ProjectViewJavaPathSection?,
/** bazel flags added to all bazel command invocations */
val buildFlags: ProjectViewBuildFlagsSection?,
/** build manual targets to build, run and test by explicitly asking. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this flag will be used only for building, for testing / running we will create another one probably.

so it could be flag for building manual targets

+ " bazel path: {},"
+ " debugger address: {},"
+ " java path: {}."
+ "manual targets : {}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its build manual targets

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+u can add build flags, since i forgot about it

extends ProjectViewSingletonSectionParser<Boolean, ProjectViewManualTargetsSection> {

public ProjectViewManualSectionParser() {
// TODO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why TODO, u can use the static name from the section


private fun combineManualSection(importedProjectViews: List<ProjectView>): ProjectViewManualTargetsSection? =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be combileBuildManualTargetsSection

Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+please undo the formatting

import org.jetbrains.bsp.bazel.projectview.model.ProjectView
import java.nio.file.Files
import java.nio.file.Path

class DefaultProjectViewGenerator : ProjectViewGenerator {

override fun generatePrettyStringAndSaveInFile(projectView: ProjectView, filePath: Path): Try<Void> =
writeStringToFile(filePath, generatePrettyString(projectView))
writeStringToFile(filePath, generatePrettyString(projectView))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, could u rollback formatting?

@basakkapusuzoglu basakkapusuzoglu deleted the issue-40 branch May 30, 2022 09:23
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.

None yet

2 participants