Feat: add support for checking if source folder exists #463
Conversation
- rm unnecessary tests
| } | ||
|
|
||
| export const checkParameters = (action: ActionInterface): void => { | ||
| if (!hasRequiredParameters(action, ['accessToken', 'gitHubToken', 'ssh'])) { |
src/constants.ts
Outdated
| @@ -40,6 +41,7 @@ export interface ActionInterface { | |||
| repositoryPath?: string | |||
| /** The root directory where your project lives. */ | |||
| root: string | |||
There was a problem hiding this comment.
Is root used anymore? We default it in the action interface but it doesn't seem to be used for anything after the more recent changes. It's also not a required parameter when using it as a node module either: https://github.com/JamesIves/github-pages-deploy-action#install-as-a-node-module-
There was a problem hiding this comment.
github-pages-deploy-action/src/git.ts
Lines 249 to 253 in d99929c
It seems that this is the only place root is used. Maybe it should be replaced by action.workplace.
There was a problem hiding this comment.
I think that makes sense, especially if rootPath and action.workspace are generating the same path anyway.
| } ${ | ||
| !fs.existsSync(`${folderPath}/.nojekyll`) | ||
| !fs.existsSync(`${action.folderPath}/.nojekyll`) |
There was a problem hiding this comment.
Will action.folderPath be defined here? I see it's assigned in the init function but I'm not sure it will be here because it's not available on action as these are not assigned to the settings object in run.
There was a problem hiding this comment.
action.folderPath will be defined here.
const settings = {
...action,
...configuration
}
await init(settings)
status = await deploy(settings)Object settings is passed by reference in function init and then passed to deploy().
| @@ -30,10 +29,6 @@ export default async function run( | |||
| ...configuration | |||
| } | |||
|
|
|||
| // Defines the repository paths and token types. | |||
| settings.repositoryPath = generateRepositoryPath(settings) | |||
| settings.tokenType = generateTokenType(settings) | |||
There was a problem hiding this comment.
Is there a purpose for moving these over to the git init function instead of leaving it here? run is the initiator so I think they make more sense here.
There was a problem hiding this comment.
I get your point. So this way I'm not sure where checkParameters should be.
There was a problem hiding this comment.
I think here would make sense right?
There was a problem hiding this comment.
Yes. I need to refactor some tests too.
Refactor: rm `action.root`, `action.rootPath` Refactor: `generateFolderPath`, `hasRequiredParameters` Test: rm some tests for `init`, add tests for `checkParameters`
Codecov Report
@@ Coverage Diff @@
## dev #463 +/- ##
===========================================
- Coverage 100.00% 96.48% -3.52%
===========================================
Files 5 5
Lines 201 199 -2
Branches 52 49 -3
===========================================
- Hits 201 192 -9
- Misses 0 6 +6
- Partials 0 1 +1
Continue to review full report at Codecov.
|
|
I am thinking of a way to fix the code coverage. |
There was a problem hiding this comment.
PR looks good! I think creating a new test and mocking the execute function and having it return an error in an error would probably be what I'd do: https://stackoverflow.com/a/50656680/4933483
I'm happy to fix the test coverage if you'd like to call this done! Let me know.
Ok I think I've done my part 🙂 |
|
Awesome, thank you again! I'll try and get this out this evening once I'm done with my testing. |
Resolves #310
Description
Refactor function
initto generate paths and check if required parameters are provided.Remove some tests.