Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

Add Generate Build File and Refresh actions#84

Merged
worksofliam merged 10 commits intomainfrom
feature/source-orbit-actions
Oct 14, 2024
Merged

Add Generate Build File and Refresh actions#84
worksofliam merged 10 commits intomainfrom
feature/source-orbit-actions

Conversation

@SanjulaGanepola
Copy link
Copy Markdown
Member

@SanjulaGanepola SanjulaGanepola commented Oct 4, 2024

⚠️Please review and merge #83 first and then rebase this PR as this one builds on top of it⚠️

Changes

  • Add Refresh action to both impact views
  • Add imd build file generation support to server
  • Add Generate Build File inline submenu with options for Bob, Make, Impact Report, and JSON Report

image

@worksofliam Do you have a better name for the JSON Report or is this alright?

Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@SanjulaGanepola SanjulaGanepola added enhancement New feature or request vs Specific to the Visual Studio Extension labels Oct 4, 2024
Comment thread vs/client/src/extension.ts
@SanjulaGanepola SanjulaGanepola linked an issue Oct 4, 2024 that may be closed by this pull request
Comment thread vs/client/src/extension.ts
Comment thread vs/client/src/tasks.ts Outdated
}
export namespace SourceOrbitTask {
interface SourceOrbitTask extends TaskDefinition {
builder: "bob" | "make" | "json";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what about this new imd thing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. I will add that here.

@worksofliam How are these tasks run? Are these something that should be in the list of runnable tasks?
image

Comment thread vs/icon.png
Comment thread vs/package.json
"name": "Affected Objects",
"contextualTitle": "Active Editor",
"when": "vscode-sourceorbit:projectsLoaded == true"
"contextualTitle": "Active Editor"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why don't you want to delay until there is a SourceOrbit project loaded?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Rather than hiding the view, I think it made more sense to always have the view visible and use welcome text.

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we somehow indicated that these are "Affected IBM i Objects"
This view will popup when the user is doing unrelated things now, they may open a Java files and not see "affected objects"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. Renaming the view would make sense or updating the welcome text to also say "...affected IBM i objects"

@worksofliam Any thoughts on the visibility of this view?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@SanjulaGanepola I do think giving it more context with a name like 'Affected IBM i objects' is better. We should do that in this PR.

We aren't going to do this: but for context, a typical way to handle multiple views and ensure they belong to the same group (like IBM i) is use a panel, so then, when the view is moved by the user to another panel (like explorer or scm) then the title becomes 'IBM i: Affected Objects'.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@worksofliam Updated the text: 1294f4b

As for moving the view outside where it typically belongs, I do see that a contextualTitle was already added to both views which is rendered instead of the panel name:

  • Active Editor: Source Impacts
  • Source Changes: Change Impacts

Keep as is? Change to IBM i: ...? Change to Source Orbit: ...?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If no change is needed, looks good to merge 😃

Comment thread vs/server/src/setup.ts
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola Let's get this branch cleaned up with the latest commits and then I will get to review today. Thanks!

Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@SanjulaGanepola SanjulaGanepola changed the base branch from main to refactor/source-orbit-tree-item October 8, 2024 15:21
@SanjulaGanepola SanjulaGanepola changed the base branch from refactor/source-orbit-tree-item to main October 8, 2024 15:21
@SanjulaGanepola
Copy link
Copy Markdown
Member Author

@worksofliam PR is ready for review

Copy link
Copy Markdown
Member

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

So most of these changes are specifually for the Project Explorer UI. I think a majorty of the new commands make sense and I am happy with them, but I don't think our package.json file should be referencing projectExplorer. Of course, these aren't hard dependencies on Project Explorer, but it becomes soft-circular when we do this.

I personally think any reference to projectExplorer in package.json should be removed (notably the new submenu entry and item/context item) and instead should be added in the Project Explorer extension.

Other than that, a lot of this makes sense to me.

@SanjulaGanepola
Copy link
Copy Markdown
Member Author

SanjulaGanepola commented Oct 8, 2024

I personally think any reference to projectExplorer in package.json should be removed (notably the new submenu entry and item/context item) and instead should be added in the Project Explorer extension.

The problem is that Project Explorer does not include Source Orbit as a hard dependency. This means that moving these commands to PE's package.json would break it when Source Orbit is not installed (unless we wrapper these same commands with PE commands).

Also, does it make sense for the tree items to be coming from the Source Orbit extension, but the command contributions are coming from PE?

What do you think?

@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola on second thought, as long as this PR doesn't add a hard dependency on anything (like PE) then I think it will be ok.

Sound ok?

@SanjulaGanepola
Copy link
Copy Markdown
Member Author

Yup I agree no hard dependency is the way to go. It should work with or without PE installed

Copy link
Copy Markdown
Member

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

I think we're almost done. Minor details now.

I do think giving it more context with a name like 'Affected IBM i objects' is better. We should do that in this PR.

Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@worksofliam worksofliam merged commit 90c9c75 into main Oct 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request vs Specific to the Visual Studio Extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add missing CLI tools to extension tree view

3 participants