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

.sgdignore and .sgdinclude could be fetched automatically if they exist in the root of an sfdx project #552

Open
AllanOricil opened this issue Mar 31, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@AllanOricil
Copy link
Contributor

AllanOricil commented Mar 31, 2023

Is your proposal related to a problem?

All the major CLIs we use look for predefined file names. For instance, .gitignore for git, webpack.config for webpack, .eslitconfig and .eslintignore for eslint, .prettierignore for prettier, pom.xml for maven, .npmignore for npm and a lot more. Therefore I think sgd could do the same for .sgdignore (-i) and .sgdinclude (-n).

steps to reproduce:

  1. create a brand new sfdx project and commit
  2. create a dummy lwc component and commit
  3. run mkdir output
  4. create a .sgdignore and add the following entry **/lwc/** in it
  5. run sfdx sgd:source:delta -f HEAD~
  6. verify the lwc is included in output/package/package.xml
  7. run sfdx sgd:source:delta -f HEAD~ -i .sgdignore`
  8. verify the lwc was excluded from output/package/package.xml
  9. confirm that sgd doesn't automatically look for a .sgdignore file in the project.

do the same steps for .sgdinclude to reach the same conclusion.

proposal

When -i is omitted, sgd must look for an .sgdignore file in the root of the project. When sgd can't find this file, it must not ignore anything.

When -n is omitted, sgd must look for an .sgdinclude file in the root of the project. When sgd can't find this file, it must not include anything.

@AllanOricil AllanOricil added the enhancement New feature or request label Mar 31, 2023
@AllanOricil AllanOricil changed the title .sgdingore and .sgdinclude must be fetched automatically if they exist in the root of an sfdx project .sgdingore and .sgdinclude could be fetched automatically if they exist in the root of an sfdx project Mar 31, 2023
@scolladon scolladon changed the title .sgdingore and .sgdinclude could be fetched automatically if they exist in the root of an sfdx project .sgdignore and .sgdinclude could be fetched automatically if they exist in the root of an sfdx project Mar 31, 2023
@scolladon
Copy link
Owner

Hi @AllanOricil !

Thanks for this enhancement request, very clear, and thanks for contributing in making this project better!

We'll have a deep look soon and let you know our thought here

@scolladon
Copy link
Owner

scolladon commented Apr 4, 2023

Hi @AllanOricil,

We have reviewed this feature request and we really like it.
That being said, as it is not related to an issue, we think it is not our current priority to work on it but we definitely think it would be a good idea to have it. We have a lot on our plate currently with the evolution we are trying to bring in the plugin and we want to make it land soon

This feature request would require

  • a section in the README to explain this new behavior
  • a new explanation message for the parameters --ignore and --include (in README and in --help output)
  • an ADR
  • unit test related to the new feature
  • integration test related to the new feature
  • NUT test related to the new behavior (optional?)

I don't think this feature request would require an ADR.

Open question: should we also consider this for the --ignore-destructive and --include-destructive parameters ? I would tend to say yes, to be consistent.
Then which name to use ?

  • .sgddestructiveignore and .sgddestructiveinclude => follows the ignore convention (my favorite)
  • .sgdignoredestructive and .sgdincludedestructive => follows the name of the parameter

Let's see when we could work on it, I cannot think of earlier than end of april / begin of may

@AllanOricil
Copy link
Contributor Author

AllanOricil commented Apr 4, 2023

@scolladon
Copy link
Owner

Yes, I feel it would be too much file to handle as well.
I like the idea of using a config file.

It means the UX will be :

  1. successful CLI parameter (if it maps to an interpretable file, otherwise log a warning) are taken first (it overrides everything)
  2. successful config attribut (if it maps to an interpretable file, otherwise log a warning) in the config file are taken second
  3. default ignore file (.sgdignore) exist ? then it is read (if it maps to an interpretable file, otherwise log a warning)
  4. if any of the precedent were configured and every of them did not map to an interpretable file (no fallback) then log an error

The actual behavior of the cli is to fail if the ignore file parameter does not exist. We need to be consistent with that to be compatible with previous UX

Then we should ask ourself what parameters we would like to handle in the config file.
To start, we should handle only those 4 in the conf file (IMHO):

  • --ignore
  • --ignore-destructive
  • --include
  • --include-destructive

Maybe more, I actually don't have strong opinion, this is something that could be driven by the community.

I think the design will need an ADR then, pointing to this feature request for solution design tracking. I update my previous comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants