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

Squash on Green #164

Merged
merged 2 commits into from
Sep 11, 2020
Merged

Squash on Green #164

merged 2 commits into from
Sep 11, 2020

Conversation

ashfurrow
Copy link
Contributor

This PR re-applies the work from #161 (reverting #163) and fixing the syntax issue that caused the revert in the first place in b366023. I pinned the TypeScript version to what's actually used by Peril to avoid this kind if unsupported-syntax-error from happening in the future.

This pins the TypeScript version to the same one used by Peril, which is ultimately what executes the code. This should prevent using unsupported syntax in the future.
Copy link
Member

@jonallured jonallured left a comment

Choose a reason for hiding this comment

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

LGTM!! 🎉

"ts-node": "^7.0.1",
"typescript": "^3.1.6"
"typescript": "3.4.1"
Copy link
Member

Choose a reason for hiding this comment

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

question [non-blocking]: typescript version pin

This is the pin you mentioned? Couldn't one clobber this with yarn upgrade --latest? Is there another, more resilient way to specify which version of a package one wants? 🤔

format via Conventional Comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the pin, I'm not sure about the semantics of yarn upgrade – I'm basing this off Renovate's PRs to pin dependencies in Eigen, which follow this format. (Previously, in c2cdda6, this was at 4.0.2.)

@jonallured jonallured merged commit b057616 into master Sep 11, 2020
@ashfurrow ashfurrow deleted the squash-on-green-fix branch September 11, 2020 18:54
@ashfurrow
Copy link
Contributor Author

Ugh! Still seeing a failure:

2020-09-11T19:03:11.993343+00:00 app[web.1]: Fri, 11 Sep 2020 19:03:11 GMT runFromSameHost Unable to evaluate the Dangerfile
2020-09-11T19:03:11.993345+00:00 app[web.1]:  TypeError: Cannot convert undefined or null to object
2020-09-11T19:03:11.993347+00:00 app[web.1]:     at Function.values (<anonymous>)
2020-09-11T19:03:11.993348+00:00 app[web.1]:     at Object.exports.rfc10 (peril-downloaded-artsy/peril-settings@org/mergeOnGreen.ts:26:35)
2020-09-11T19:03:11.993349+00:00 app[web.1]:     at process._tickCallback (internal/process/next_tick.js:68:7)

I'll have to look into this later, sorry for the noise Jon!

@jonallured
Copy link
Member

Oh bummer - no problem though we can totally come back to this. Monday Ash is going to crush it!! ❤️

@ashfurrow
Copy link
Contributor Author

My guess is that the relative import between files isn't setup right (because of how Peril executes things). Other relative imports do a weird await import dance that we need to replicate.

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.

2 participants