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

fix(windows): add UseBasicParsing flag #49

Merged
merged 1 commit into from
Jan 5, 2021
Merged

Conversation

erezrokah
Copy link
Contributor

@erezrokah erezrokah added the type: bug code to address defects in shipped code label Dec 23, 2020
Copy link
Contributor

@keiko713 keiko713 left a comment

Choose a reason for hiding this comment

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

wow this issue looks extremely familiar to me for some reason. I think we did have this issue when we initially launched but not sure why it's not fixed 🤔 looks good to me

@keiko713
Copy link
Contributor

keiko713 commented Jan 5, 2021

uhmm I think we also want to ideally move away from azure devops to github actions...

@erezrokah
Copy link
Contributor Author

erezrokah commented Jan 5, 2021

wow this issue looks extremely familiar to me for some reason

Maybe because of https://github.com/netlify/netlify-lm-plugin/blob/f129b0ccb90fefd2226cf1f468797f12708a2bf4/src/install.ts#L96 and netlify/netlify-lm-plugin@6faf095 :)

looks good to me

Does this mean I can merge the PR?

@keiko713
Copy link
Contributor

keiko713 commented Jan 5, 2021

Maybe because of...

ohhh yes that's it, thanks for finding!

Does this mean I can merge the PR?

yes! let's merge it and think about github action later. you may need a help from @calavera for how to release though

@erezrokah erezrokah merged commit 53fbf6e into master Jan 5, 2021
@erezrokah erezrokah deleted the fix/windows_install branch January 5, 2021 16:44
@erezrokah
Copy link
Contributor Author

you may need a help from @calavera for how to release though

We install directly from master:
https://github.com/netlify/netlify-lm-plugin/blob/f129b0ccb90fefd2226cf1f468797f12708a2bf4/src/install.ts#L96
so I think we don't need to release anything

@keiko713
Copy link
Contributor

keiko713 commented Jan 5, 2021

it's indeed no necessary from the netlify lm plugin perspective as you pointed it out, but for other ways to install (like mentioned in README), release will be needed, and we've been making releases:
https://github.com/netlify/netlify-credential-helper/releases
#43

how about we don't cut the release for this for now, and cut the release once the migration from azure devops to github actions is done? (aka you don't need to worry about it for now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants