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

Jump to bottom

Only warn about auto-regeneration bug on Bash On Windows. #5464

Merged
merged 2 commits into from Nov 10, 2016
Merged

Only warn about auto-regeneration bug on Bash On Windows. #5464

merged 2 commits into from Nov 10, 2016

Conversation

envygeeks
Copy link
Contributor

No description provided.

@parkr
Copy link
Member

parkr commented Oct 7, 2016

/cc @jekyll/windows – is there someone who would be willing to try this out?

@ashmaroli
Copy link
Member

Do you mean test it out..? I'll volunteer..

@parkr
Copy link
Member

parkr commented Oct 7, 2016 •
edited

Do you mean test it out..? I'll volunteer..

Yep. Thanks!

@parkr parkr added fix windows Related to the Windows platform labels Oct 7, 2016
@ashmaroli
Copy link
Member

Test passed.. was able to view regenerated site successfully..

parkr
parkr reviewed Oct 7, 2016
View reviewed changes
lib/jekyll/commands/build.rb
"https://github.com/Microsoft/BashOnWindows/issues/216"
if Utils::Platforms.bash_on_windows?
Jekyll.logger.warn "", "--watch arg is unsupported in Bash on Windows. "
Jekyll.logger.warn "", "Please see: https://github.com/Microsoft/BashOnWindows/issues/216"
Copy link
Member

@parkr parkr Oct 7, 2016 •
edited

Choose a reason for hiding this comment

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

Looks like this line is too long per the fmt build failure.

Copy link
Member

Choose a reason for hiding this comment

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

*fmt 👆

Copy link
Member

Choose a reason for hiding this comment

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

Yep. I review on my phone (with autocorrect) a lot.

Daniel15
Daniel15 reviewed Oct 9, 2016
View reviewed changes
lib/jekyll/commands/build.rb
if Utils::Platforms.bash_on_windows?
Jekyll.logger.warn "", "--watch arg is unsupported in Bash on Windows. "
Jekyll.logger.warn "", "Please see: https://github.com/Microsoft/BashOnWindows/issues/216"
Jekyll.logger.warn "", "If iNotify is fixed, please file a ticket."
Copy link

Choose a reason for hiding this comment

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

inotify (all lowercase). It's not an Apple product 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll care about that when people start saying GNU/Linux instead of Linux. ;)

@Daniel15
Copy link

Daniel15 commented Oct 9, 2016

Thanks! This fixes watching for me on Windows.

parkr
parkr approved these changes Oct 24, 2016
View reviewed changes
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

Thank you! We can ship this as v3.3.1.

@parkr
Copy link
Member

parkr commented Oct 24, 2016

Fixes #5462.

@DirtyF DirtyF added this to the 3.3.1 milestone Oct 24, 2016
@envygeeks
Copy link
Contributor Author

I'll fix the fmt error in the morning.

@envygeeks
Copy link
Contributor Author

oh wait no, I was supposed to remove this entirely, BashOnWindows will support iNotify in the next release.

@envygeeks
Copy link
Contributor Author

I'll adjust the pull to reflect that in the morning.

@parkr
Copy link
Member

parkr commented Oct 25, 2016

oh wait no, I was supposed to remove this entirely, BashOnWindows will support iNotify in the next release.

Oh nice!

@ashmaroli ashmaroli mentioned this pull request Nov 7, 2016
Closed
@parkr
Copy link
Member

parkr commented Nov 9, 2016

Hey @envygeeks, what else needs to be done here?

@envygeeks
Copy link
Contributor Author

I think I need to fix the format error, I'll get it done in the morning, I've ben so busy I forgot about this.

@parkr parkr force-pushed the allow-regular-windows-watchers branch from 6cf2d31 to df45f26 Compare November 10, 2016 20:55
@parkr
Copy link
Member

parkr commented Nov 10, 2016

Moving from disabling watch altogether to just warning in case they have issues.

@parkr parkr assigned parkr and unassigned envygeeks Nov 10, 2016
@parkr parkr changed the title Fix #5462: Only shutdown watch in Bash On Windows. Only warn about auto-regeneration bug on Bash On Windows. Nov 10, 2016
@parkr
Copy link
Member

parkr commented Nov 10, 2016

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit f8ec71a into master Nov 10, 2016
@jekyllbot jekyllbot added the bug label Nov 10, 2016
@jekyllbot jekyllbot added the fix label Nov 10, 2016
@jekyllbot jekyllbot deleted the allow-regular-windows-watchers branch November 10, 2016 23:05
jekyllbot added a commit that referenced this pull request Nov 10, 2016
@jekyllbot
Update history to reflect merge of #5464 [ci skip]
07d4333
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Daniel15 Daniel15 Daniel15 left review comments

@ashmaroli ashmaroli ashmaroli left review comments

@parkr parkr parkr approved these changes

Assignees

@parkr parkr

Labels
fix frozen-due-to-age windows Related to the Windows platform
Projects
None yet
Milestone
3.3.1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@envygeeks @parkr @ashmaroli @Daniel15 @DirtyF @jekyllbot