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

feat: delta analysis scoped by source parameter #174

Merged

Conversation

sudhanshu-gupta-0704
Copy link
Contributor

@sudhanshu-gupta-0704 sudhanshu-gupta-0704 commented Jul 31, 2021

What does this pull request contains

This pull request contains code modification so that the delta can be generated for a particular module or type of metadata, such as delta only for classes/lwc/etc.
It is complementary to '--ignore' and '--ignore-destructive' and works with it

Use same message for both legacy cli and sfdx plugin

Default output folder is now '.'. See "Explain you changes" and "README.md" changes

  • Added for new features.
  • Changed for changes in existing functionality.
  • Deprecated for soon-to-be removed features.
  • Removed for now removed features.
  • Fixed for any bug fixes.
  • Security in case of vulnerabilities.

Explain your changes

--source option added in CLI to select or pass the folder path only to be taken into consideration while generating Delta, by default whole folder is taken into consideration (no change to existing functionality).

2. /!\ breaking change /!\ output default folder is now '.' (instead of 'output'). It will be taken automatically when not specified, except if delta generation ('-d') is also passed. In this case an error will be thrown because it would override the current repository with the delta generation file (To be discussed)

Does this close any currently open issues?

Closes #168

Any particular element to being able to test locally


-o has now a new default => check your integration !
-s newly added can focus the command to sub folder (see README.md changes)

Any other comments?

Where has this been tested?

on Windows, Azure DevOps Pipeline, Mac

Operating System: WINDOWS 10 21H1

**yarn version:**1.22.11

**node version:**14.17.3

**git version:**2.31.0.windows.1

**sfdx version:**7.110.0

**sgd plugin version:**4.7.2

src/main.js Outdated
@@ -29,6 +29,12 @@ const checkConfig = (config, repoSetup) => {
) {
errors.push(`${config.output} folder does not exist`)
}
if (
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

src/main.js Outdated
) {
errors.push(`${config.output} folder does not exist`)
}
this.chkIfDirExist(config.output);
Copy link

Choose a reason for hiding this comment

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

Delete ;

src/main.js Outdated
errors.push(`${config.output} folder does not exist`)
}
this.chkIfDirExist(config.output);
this.chkIfDirExist(config.source);
Copy link

Choose a reason for hiding this comment

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

Delete ;

src/main.js Outdated
@@ -43,10 +39,20 @@ const checkConfig = (config, repoSetup) => {
return errors
}

function chkIfDirExist(Dir){
Copy link

Choose a reason for hiding this comment

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

'chkIfDirExist' is defined but never used.

src/main.js Outdated
@@ -43,10 +39,20 @@ const checkConfig = (config, repoSetup) => {
return errors
}

function chkIfDirExist(Dir){
Copy link

Choose a reason for hiding this comment

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

Insert ·

src/main.js Outdated
@@ -43,10 +39,20 @@ const checkConfig = (config, repoSetup) => {
return errors
}

function chkIfDirExist(Dir){
if (
Copy link

Choose a reason for hiding this comment

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

Replace ⏎····!fs.existsSync(Dir)·||⏎····!fs.statSync(Dir).isDirectory()⏎·· with !fs.existsSync(Dir)·||·!fs.statSync(Dir).isDirectory()

src/main.js Outdated
if (!repoSetup.isToEqualHead() && config.generateDelta) {
errors.push(
`--generate-delta (-d) parameter cannot be used when --to (-t) parameter is not equivalent to HEAD`
)
errors=chkIfDirExist(config.output, errors)
Copy link

Choose a reason for hiding this comment

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

Replace errors= with ··errors·=·

src/main.js Outdated
if (!repoSetup.isToEqualHead() && config.generateDelta) {
errors.push(
`--generate-delta (-d) parameter cannot be used when --to (-t) parameter is not equivalent to HEAD`
)
errors=chkIfDirExist(config.output, errors)
Copy link

Choose a reason for hiding this comment

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

'errors' is constant.

src/main.js Outdated
if (!repoSetup.isToEqualHead() && config.generateDelta) {
errors.push(
`--generate-delta (-d) parameter cannot be used when --to (-t) parameter is not equivalent to HEAD`
)
errors=chkIfDirExist(config.output, errors)
errors=chkIfDirExist(config.source, errors)
Copy link

Choose a reason for hiding this comment

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

Replace ··errors= with ····errors·=·

src/main.js Outdated
if (!repoSetup.isToEqualHead() && config.generateDelta) {
errors.push(
`--generate-delta (-d) parameter cannot be used when --to (-t) parameter is not equivalent to HEAD`
)
errors=chkIfDirExist(config.output, errors)
errors=chkIfDirExist(config.source, errors)
Copy link

Choose a reason for hiding this comment

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

'errors' is constant.

src/main.js Outdated
}

return errors
}

function chkIfDirExist(dir,errors){
Copy link

Choose a reason for hiding this comment

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

Replace errors) with ·errors)·

@sudhanshu-gupta-0704 sudhanshu-gupta-0704 changed the title Sudhanshu/add modular support Modular support for Delta Generation Jul 31, 2021
@scolladon scolladon self-requested a review August 1, 2021 10:27
@scolladon scolladon self-assigned this Aug 1, 2021
@scolladon
Copy link
Owner

Hi @sudhanshu-gupta-0704 !

Thanks for this contribution ! That is great.
I'm currently reviewing it.

Could you give me access to your fork please ? So I can add commits to the PR

@sudhanshu-gupta-0704
Copy link
Contributor Author

Hi @scolladon ,

I have added you as a contributor to my fork, Please use Sudhanshu/AddModularSupport branch for changes.

Thanks

src/utils/cliHelper.js Show resolved Hide resolved
src/utils/cliHelper.js Show resolved Hide resolved
src/utils/cliHelper.js Outdated Show resolved Hide resolved
src/utils/cliHelper.js Outdated Show resolved Hide resolved
src/utils/cliHelper.js Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 1, 2021

Codecov Report

Merging #174 (90b0eb6) into master (dda6327) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #174   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          18       19    +1     
  Lines         440      450   +10     
=======================================
+ Hits          439      449   +10     
  Misses          1        1           
Impacted Files Coverage Δ
src/utils/repoGitDiff.js 100.00% <ø> (ø)
src/main.js 100.00% <100.00%> (ø)
src/service/standardHandler.js 100.00% <100.00%> (ø)
src/utils/childProcessUtils.js 100.00% <100.00%> (ø)
src/utils/cliHelper.js 100.00% <100.00%> (ø)

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 dda6327...90b0eb6. Read the comment docs.

@scolladon scolladon changed the title Modular support for Delta Generation feat: delta analysis scoped by source parameter Aug 1, 2021
src/utils/cliHelper.js Show resolved Hide resolved
src/utils/cliHelper.js Show resolved Hide resolved
@scolladon
Copy link
Owner

scolladon commented Aug 1, 2021

FYI: PR Lint issue is false positive

Issue is related to GITHUB_TOKEN not accessible because PR is created from a fork (cf known issue)
The PR name is correct following our convention

@codeclimate
Copy link

codeclimate bot commented Aug 4, 2021

Code Climate has analyzed commit 90b0eb6 and detected 0 issues on this pull request.

View more on Code Climate.

@scolladon scolladon enabled auto-merge (squash) August 4, 2021 14:01
@scolladon scolladon disabled auto-merge August 4, 2021 14:01
@scolladon scolladon merged commit 62f0cf2 into scolladon:master Aug 4, 2021
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.

QUESTION: Multiple Package Directories
3 participants