Skip to content

[Minor]ignore all config files in conf #2395

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

Closed
wants to merge 2 commits into from
Closed

Conversation

scwf
Copy link
Contributor

@scwf scwf commented Sep 15, 2014

Some config files in conf should ignore, such as
conf/fairscheduler.xml
conf/hive-log4j.properties
conf/metrics.properties
...
So ignore all sh/properties/conf/xml files

@SparkQA
Copy link

SparkQA commented Sep 15, 2014

Can one of the admins verify this patch?

@pwendell
Copy link
Contributor

Jenkins, test this please.

@pwendell
Copy link
Contributor

LGTM pending tests.

@SparkQA
Copy link

SparkQA commented Sep 15, 2014

QA tests have started for PR 2395 at commit 3dc53f2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 15, 2014

QA tests have finished for PR 2395 at commit 3dc53f2.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sarutak
Copy link
Member

sarutak commented Sep 16, 2014

@scwf Thanks for your work!
Also how about adding following files to .gitignore?

  • conf/slaves
  • conf/*.template

@scwf
Copy link
Contributor Author

scwf commented Sep 16, 2014

@sarutak , i think we should not add them to .gitignore, this files is the sample file we show to users, in future devs may change them according to updating of master branch.

@sarutak
Copy link
Member

sarutak commented Sep 16, 2014

@scwf Ah, exactly, .template is just a sample. But slaves is a kind of confs right?

@scwf
Copy link
Contributor Author

scwf commented Sep 16, 2014

@sarutak , ok, i will add it

@srowen
Copy link
Member

srowen commented Sep 16, 2014

Hang on, that file is versioned in the repo. I don't think you want to ignore it! Not without deciding it should be deleted.

@scwf
Copy link
Contributor Author

scwf commented Sep 16, 2014

yeah, @sarutak, ignore slaves means that it will not be versioned, so not ignore it

@sarutak
Copy link
Member

sarutak commented Sep 16, 2014

O.K. I got it.

@pwendell
Copy link
Contributor

@srowen was your comment about something in the current patch or proposed in one of the comments? Does this LGTY?

@srowen
Copy link
Member

srowen commented Sep 16, 2014

I was commenting on a comment, suggesting to also ignore conf/slaves. It is not in the PR so LGTM.

@pwendell
Copy link
Contributor

Jenkins LGTM.

@pwendell
Copy link
Contributor

Jenkins, test this please.

@andrewor14
Copy link
Contributor

LGTMT

@pwendell
Copy link
Contributor

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have started for PR 2395 at commit 3dc53f2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have started for PR 2395 at commit 3dc53f2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have finished for PR 2395 at commit 3dc53f2.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class ArrayConstructor extends net.razorvine.pickle.objects.ArrayConstructor
    • class NonASCIICharacterChecker extends ScalariformChecker
    • class SCCallSiteSync(object):

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have finished for PR 2395 at commit 3dc53f2.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@pwendell
Copy link
Contributor

Merged, thanks.

@asfgit asfgit closed this in 008a5ed Sep 17, 2014
@scwf scwf deleted the patch-2 branch September 24, 2014 08:13
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.

6 participants