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

stop metadata.mds service if running in osx #490

Closed
wants to merge 1 commit into from

Conversation

@vigneshsarma
Copy link

vigneshsarma commented Sep 24, 2016

for issue #445

@aneeshusa creating the PR, to see the tests run.


This change is Reviewable

@aneeshusa
Copy link
Member

aneeshusa commented Sep 24, 2016

Thanks for the PR! The commit itself looks great, so here is some review to improve the commit message (see also):

Title:

  • Start with a capital letter
  • stop -> Disable
  • metadata.mds -> metadata indexing service
  • if running in osx -> on OS X

Body:

  • Instead of for issue #445, please include the actual reasoning for this commit from the issue. Why is this commit being made? What problem does it solve? Include details from the IRC convo linked in the commit. (The goal is to have the body by itself include the relevant details so it is useful offline, e.g. in git log.)
Metadata indexing Service on OS X, indexes files. This causes regular
CPU spikes and bad performance on build servers. So disabling the
service.
@vigneshsarma
Copy link
Author

vigneshsarma commented Sep 24, 2016

@aneeshusa What do you think now?

I am no good at writing these kind of stuff. Hope its ok 😄

@aneeshusa
Copy link
Member

aneeshusa commented Sep 27, 2016

This is better but there's still room for improvement:

  • Good that you included info from IRC about CPU usage, but let's be even more specific: "taking 100% of a CPU" is a quote that has an illustrative statistic
  • Make sure to use complete sentences

If you're having trouble with the wording, you can also give me access to push to your branch with some suggestions.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 27, 2016

The latest upstream changes (presumably #483) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm jdm closed this Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.