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.

Sign up for GitHub

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

Use readFileSync instead of require #101

Merged
merged 2 commits into from
Aug 30, 2019
Merged

Use readFileSync instead of require #101

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b9cfea
Use readFileSync instead of require
Aug 30, 2019
3a16ee7
error handling
Aug 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions packages/github/src/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Originally pulled from https://github.com/JasonEtco/actions-toolkit/blob/master/src/context.ts
import {WebhookPayload} from './interfaces'

/* eslint-disable @typescript-eslint/no-require-imports */
import {readFileSync, existsSync} from 'fs'
import {EOL} from 'os'

export class Context {
/**
Expand All @@ -20,9 +20,20 @@ export class Context {
* Hydrate the context from the environment
*/
constructor() {
this.payload = process.env.GITHUB_EVENT_PATH
? require(process.env.GITHUB_EVENT_PATH)
: {}
this.payload = {}
if (process.env.GITHUB_EVENT_PATH) {
if (existsSync(process.env.GITHUB_EVENT_PATH)) {
this.payload = JSON.parse(
readFileSync(process.env.GITHUB_EVENT_PATH, {encoding: 'utf8'})
Copy link

Choose a reason for hiding this comment

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

Do we want to add error handling here? Make sure the file exists first and print message if not?

Copy link

Choose a reason for hiding this comment

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

And while we are here, should we debug print a serialization of the context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error handling yes, serialization of the context seems like a no to me. Its pretty massive, and since we're built to be a dependency I'm a little leary of large debug output that may or may not be helpful, especially since lots of actions probably won't use that directly.

)
} else {
process.stdout.write(
`GITHUB_EVENT_PATH ${
process.env.GITHUB_EVENT_PATH
} does not exist${EOL}`
)
}
}
this.eventName = process.env.GITHUB_EVENT_NAME as string
this.sha = process.env.GITHUB_SHA as string
this.ref = process.env.GITHUB_REF as string
Expand Down