Skip to content

Evented File System Monitoring#20785

Closed
puneet24 wants to merge 6 commits into
rails:masterfrom
puneet24:puneet
Closed

Evented File System Monitoring#20785
puneet24 wants to merge 6 commits into
rails:masterfrom
puneet24:puneet

Conversation

@puneet24

@puneet24 puneet24 commented Jul 5, 2015

Copy link
Copy Markdown

No description provided.

@kaspth kaspth added the GSoC label Jul 5, 2015
@kaspth kaspth added this to the 5.0.0 milestone Jul 5, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like you're indentation is off. Can you switch to use 2 spaces as the indentation unit throughout? 😄

It would look like this:

module ActiveSupport
  class FileEventedUpdateChecker
    attr_reader :listener

    # The rest of your code...
  end
end

Remember to strip whitespace at the end of every line too (and empty lines should have no spaces) 😄

@puneet24

puneet24 commented Jul 5, 2015

Copy link
Copy Markdown
Author

I have made indentation according to convention.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please remove this indentation?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes I missed this one. I will remove that,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO this comment needs to be more descriptive as to why / how performance will be improved - especially because in some environments (Windows, for instance) the "fast" version requires an additional gem.

@thibaudgg

Copy link
Copy Markdown

Really excited to see Listen being used by Rails! ❤️

@rymai

rymai commented Sep 28, 2015

Copy link
Copy Markdown
Contributor

That's awesome! 💥

@fxn

fxn commented Sep 28, 2015

Copy link
Copy Markdown
Member

Hi @puneet24 could you please resolve conflicts here?

@puneet24

Copy link
Copy Markdown
Author

Hi @fxn, I have resolved the conflicts in latest commit.

@fxn

fxn commented Sep 30, 2015

Copy link
Copy Markdown
Member

@puneet24 thanks! GitHub still says this branch has conflicts... maybe you updated a different branch?

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.

9 participants