Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Feature/eslint prettier #1081

Merged
merged 6 commits into from
Jan 16, 2019
Merged

Feature/eslint prettier #1081

merged 6 commits into from
Jan 16, 2019

Conversation

honestbonsai
Copy link
Contributor

Resolves #1080

@adrianmcli
Copy link
Contributor

noice, lgtm

@adrianmcli
Copy link
Contributor

@honestbonsai not sure why the PR build failed. Looks like an issue with the windows build? I'm inclined to just merge it anyway since the branch build was fine, but I'm not sure.

@mikeseese
Copy link
Contributor

mikeseese commented Jan 16, 2019

I believe that appveyor has issues signing on the pr build. You need to investigate what the error is before "inclining to merge". The error was related to signing, and the other appveyor build was successful, which I would consider sufficient

@adrianmcli
Copy link
Contributor

adrianmcli commented Jan 16, 2019

You need to investigate what the error is before "inclining to merge".

@seesemichaelj thanks for taking a look. I did investigate as I mentioned, but I couldn't make sense of all the windows noise.

Error: Cannot extract publisher name from code signing certificate

I don't know how windows builds work, but if the branch build was ok I don't see how this shouldn't be either. Why is this not deterministic?

@adrianmcli adrianmcli merged commit 93f9eca into develop Jan 16, 2019
@mikeseese
Copy link
Contributor

thanks for taking a look. I did investigate as I mentioned, but I couldn't make sense of all the windows noise.

@adrianmcli haha, I usually just read the first red line

I don't know how windows builds work, but if the branch build was ok I don't see how this shouldn't be either. Why is this not deterministic?

I'm not sure actually. I didn't setup the build processes and didn't take the time to figure it out. I'm guessing some signing env variable is not set properly. If you look at the history of appveyor.yml there was a commit about getting PR builds to get fixed from @benjamincburns. Perhaps the solution was there and got removed/changed over the last few months 🤷‍♂️

@adrianmcli
Copy link
Contributor

We really need a dedicated electron guy on this.

@mikeseese mikeseese deleted the feature/eslint-prettier branch January 16, 2019 17:05
@honestbonsai
Copy link
Contributor Author

honestbonsai commented Jan 16, 2019

Thanks! Good to know about the keys and signing breaking PR builds.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants