add file support #119
add file support #119
Conversation
There was a problem hiding this comment.
Thanks for your contribution @EhsanHerai !
A bunch of comments of linting, docs and strategy around message > filePath
| const github_token: string = core.getInput('GITHUB_TOKEN'); | ||
| const pr_number: string = core.getInput('pr_number'); | ||
| const comment_includes: string = core.getInput('comment_includes'); | ||
| const reactions: string = core.getInput('reactions'); | ||
|
|
||
| if(!filePath && !message) { |
There was a problem hiding this comment.
| if(!filePath && !message) { | |
| if (!filePath && !message) { |
| if(filePath) { | ||
| if(!process.env.GITHUB_WORKSPACE) { |
There was a problem hiding this comment.
| if(filePath) { | |
| if(!process.env.GITHUB_WORKSPACE) { | |
| if (filePath) { | |
| if (!process.env.GITHUB_WORKSPACE) { |
| const github_token: string = core.getInput('GITHUB_TOKEN'); | ||
| const pr_number: string = core.getInput('pr_number'); | ||
| const comment_includes: string = core.getInput('comment_includes'); | ||
| const reactions: string = core.getInput('reactions'); | ||
|
|
||
| if(!filePath && !message) { | ||
| throw new Error('either filePath or message input should be provided!'); |
There was a problem hiding this comment.
| throw new Error('either filePath or message input should be provided!'); | |
| throw new Error('Either "filePath" or "message" input should be provided'); |
|
|
||
| if(filePath) { | ||
| if(!process.env.GITHUB_WORKSPACE) { | ||
| throw new Error('GITHUB_WORKSPACE is not set! please make sure to use action/checkout action!'); |
There was a problem hiding this comment.
| throw new Error('GITHUB_WORKSPACE is not set! please make sure to use action/checkout action!'); | |
| throw new Error('GITHUB_WORKSPACE is not set, please make sure to use actions/checkout action'); |
| const _path = path.join(process.env.GITHUB_WORKSPACE, filePath); | ||
| message = fs.readFileSync(_path, 'utf8'); |
There was a problem hiding this comment.
| const _path = path.join(process.env.GITHUB_WORKSPACE, filePath); | |
| message = fs.readFileSync(_path, 'utf8'); | |
| const absolutefilePath = path.join(process.env.GITHUB_WORKSPACE, filePath); | |
| message = fs.readFileSync(absolutefilePath, 'utf8'); |
| required: true | ||
| required: false | ||
| filePath: | ||
| description: 'File that should be commented' |
There was a problem hiding this comment.
| description: 'File that should be commented' | |
| description: 'File content that should be commented' |
| @@ -8,12 +10,25 @@ type Reaction = typeof REACTIONS[number]; | |||
|
|
|||
| async function run() { | |||
| try { | |||
| const message: string = core.getInput('message'); | |||
| let message: string = core.getInput('message'); | |||
There was a problem hiding this comment.
| let message: string = core.getInput('message'); | |
| const message: string = core.getInput('message'); |
I'd prefer here to let the input variable let as it is.
Prefer creating a new variable that would be something like : body (defaults to = message) and override this one in case of the presence of fielPath
| @@ -28,6 +28,25 @@ jobs: | |||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |||
| ``` | |||
|
|
|||
| ### specify a file | |||
There was a problem hiding this comment.
| ### specify a file | |
| ### Specify a file |
Here IMHO, we should also specify that in case this varibale is filled up, we override the message.
For instance :
- name: Comment PR
uses: thollander/actions-comment-pull-request@v1
with:
message: Hello !
filePath: /path/to/file.txt
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Would go for the file and not the message.
Should it really be the default behaviour btw ? I think message should have more power than filePath in case it's present. Then I would go for an if (filePath && !message)
No description provided.