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

Easy Deploy on Save #822

Merged
merged 27 commits into from
Dec 20, 2018
Merged

Easy Deploy on Save #822

merged 27 commits into from
Dec 20, 2018

Conversation

allileong
Copy link
Contributor

@allileong allileong commented Dec 14, 2018

What does this PR do?

When the workspace setting for deploy-on-save is enabled, changes made to files inside of a package directory will trigger an automatic push or deploy, depending on the type of org that is connected to the workspace.

Note: This functionality is not working in Windows due to a problem with the GlobPattern and I have followed the following issue with VS Code: microsoft/vscode#65240

What issues does this PR fix or reference?

W-5552627, #577, #662

@codecov
Copy link

codecov bot commented Dec 15, 2018

Codecov Report

Merging #822 into develop will decrease coverage by 0.07%.
The diff coverage is 73.52%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #822      +/-   ##
===========================================
- Coverage    73.26%   73.18%   -0.08%     
===========================================
  Files          168      174       +6     
  Lines         6815     6997     +182     
  Branches      1076     1094      +18     
===========================================
+ Hits          4993     5121     +128     
- Misses        1550     1599      +49     
- Partials       272      277       +5
Impacted Files Coverage Δ
...rcedx-vscode-core/src/settings/sfdxCoreSettings.ts 57.14% <0%> (-4.4%) ⬇️
...ackages/salesforcedx-vscode-core/src/util/index.ts 100% <100%> (ø)
...ges/salesforcedx-vscode-core/src/settings/index.ts 100% <100%> (ø) ⬆️
...ages/salesforcedx-vscode-core/src/context/index.ts 100% <100%> (ø) ⬆️
.../salesforcedx-vscode-core/src/commands/commands.ts 72.1% <20%> (-1.84%) ⬇️
...forcedx-vscode-core/src/diagnostics/diagnostics.ts 77.41% <50%> (-1.9%) ⬇️
...edx-vscode-core/src/settings/pushOrDeployOnSave.ts 60.82% <60.82%> (ø)
...e-core/src/commands/forceSourceDeploySourcePath.ts 61.11% <61.11%> (ø)
...cedx-vscode-core/src/commands/forceSourceDeploy.ts 27.58% <66.66%> (-14.92%) ⬇️
...e-core/src/commands/forceSourceRetrieveManifest.ts 75% <75%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6a526a...26af321. Read the comment docs.

@allileong allileong changed the title [Almost-Ready-For-Review] Easy Deploy on Save Easy Deploy on Save Dec 18, 2018
@@ -38,7 +38,8 @@
"args": ["--extensionDevelopmentPath=${workspaceRoot}/packages"],
"stopOnEntry": false,
"sourceMaps": true,
"outFiles": ["${workspaceRoot}/packages/*/out/src/**/*.js"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please disregard changes made to this file. I will be reverting all changes.

@allileong allileong force-pushed the alli/easyDeployRebased branch from c727e41 to c8b8c70 Compare December 19, 2018 21:38
const WAIT_TIME_IN_MS = 50;

export async function registerPushOrDeployOnSave() {
if (sfdxCoreSettings.getPushOrDeployOnSaveEnabled()) {
Copy link
Contributor Author

@allileong allileong Dec 19, 2018

Choose a reason for hiding this comment

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

We only register the file watcher if the push-or-deploy-on-save setting is already set to true on core activation. This means that if a user wants to turn on the feature, they will have to restart VS Code.

Disablement of the feature will be respected immediately due to the checks made in lines 43, 56, and 65, and will not require a restart of the extension.

@@ -1,3 +0,0 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to delete this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this. I did not mean to delete this and will put it back.


vscode.workspace.onDidChangeTextDocument(e => {
if (ForceSourceDeployExecutor.errorCollection.has(e.document.uri)) {
ForceSourceDeployExecutor.errorCollection.delete(e.document.uri);
}
});

export class ForceSourceDeployExecutor extends SfdxCommandletExecutor<
SelectedPath
export abstract class ForceSourceDeployExecutor extends SfdxCommandletExecutor<
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, this makes every type of deploy command render any deploy errors in the problem view.

const commandBuilder = new SfdxCommandBuilder()
.withDescription(nls.localize('force_source_deploy_text'))
.withArg('force:source:deploy')
.withLogName('force_source_deploy')
Copy link
Contributor

@lcampos lcampos Dec 19, 2018

Choose a reason for hiding this comment

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

Lets change this to fource_source_deploy_with_manifest

const commandBuilder = new SfdxCommandBuilder()
.withDescription(nls.localize('force_source_deploy_text'))
.withArg('force:source:deploy')
.withLogName('force_source_deploy')
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets change this to force_source_deploy_with_sourcepath to keep consistency with the log ids for the retrieve commands.

if (sourcePathIsInPackageDirectory) {
return inputs;
}
} catch (error) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should log this error and/or send it to our telemetry service.


let sourcePathIsInPackageDirectory = false;
for (const packagePath of fullPackagePaths) {
if (sourcePath.startsWith(packagePath)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't packagePath be something like force-app and sourcePath be something like Users/myuser/repo_name/force-app ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Disregard this, I've checked and I was wrong 👍

Copy link
Contributor

@lcampos lcampos left a comment

Choose a reason for hiding this comment

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

🚢

@allileong allileong merged commit a7385ab into develop Dec 20, 2018
@allileong allileong deleted the alli/easyDeployRebased branch December 20, 2018 17:11
@anandbn anandbn mentioned this pull request Jan 20, 2019
@salesforce-cla
Copy link

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Alli <a***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants