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

Use `git check-attr` to process .gitattributes' export-ignore #45

Merged
merged 1 commit into from Aug 13, 2018

Conversation

Projects
None yet
2 participants
@legoktm
Contributor

legoktm commented Jul 20, 2018

Instead of implementing our own parsing and handling of
.gitattributes, shell out to git's check-attr command to take care
of the parsing and processing for us.

Notably, git handles unsetting properties properly.

Fixes #44.

The downside of shelling out is that in my testing it was 3x slower for MediaWiki, but the upside is that it cut down a ton custom code.

Use `git check-attr` to process .gitattributes' export-ignore
Instead of implementing our own parsing and handling of
`.gitattributes`, shell out to git's `check-attr` command to take care
of the parsing and processing for us.

Notably, git handles unsetting properties properly.

Fixes #44.
return is_excluded
out = self.run_git_shell(
'git check-attr -a -- %s' % repo_file_path,

This comment has been minimized.

@Kentzo

Kentzo Jul 25, 2018

Owner

@legoktm Any reason why --all was preferred over a specific attribute? As far as I understand git check-attr export-ignore should have worked.

Also I think we should use -z and split by NUL instead of in.

Maybe we could run this command just once: git check-attr -z export-ignore -- <main_repo_abspath>or at lease just once per repo: git check-attr -z export-ignore -- <repo_abspath>. Then we can cache and reuse the result.

@Kentzo

Kentzo Jul 25, 2018

Owner

@legoktm Any reason why --all was preferred over a specific attribute? As far as I understand git check-attr export-ignore should have worked.

Also I think we should use -z and split by NUL instead of in.

Maybe we could run this command just once: git check-attr -z export-ignore -- <main_repo_abspath>or at lease just once per repo: git check-attr -z export-ignore -- <repo_abspath>. Then we can cache and reuse the result.

@Kentzo Kentzo closed this Aug 13, 2018

@Kentzo Kentzo reopened this Aug 13, 2018

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Aug 13, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@16a79f7). Click here to learn what that means.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #45   +/-   ##
=========================================
  Coverage          ?   50.67%           
=========================================
  Files             ?        1           
  Lines             ?      148           
  Branches          ?       29           
=========================================
  Hits              ?       75           
  Misses            ?       59           
  Partials          ?       14
Impacted Files Coverage Δ
git_archive_all.py 50.67% <85.71%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16a79f7...2a34ad4. Read the comment docs.

codecov bot commented Aug 13, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@16a79f7). Click here to learn what that means.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #45   +/-   ##
=========================================
  Coverage          ?   50.67%           
=========================================
  Files             ?        1           
  Lines             ?      148           
  Branches          ?       29           
=========================================
  Hits              ?       75           
  Misses            ?       59           
  Partials          ?       14
Impacted Files Coverage Δ
git_archive_all.py 50.67% <85.71%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16a79f7...2a34ad4. Read the comment docs.

@Kentzo Kentzo merged commit ee17c7b into Kentzo:master Aug 13, 2018

3 checks passed

codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment