search

ldc-developers / ldc2.snap

Snap package definition for LDC, the LLVM-based D compiler
11 stars 4 forks source link

git source-depth == 1 #100

Closed denizzzka closed 3 years ago

WebDrake commented 4 years ago

Thanks for the PR, but what's the motivation for this change?

As a general rule it's nice if all contributors can write commit messages that make clear both what has changed and why (including what the expected impact will be).

BTW I think in any case there is a small typo in your commit-title as-is :-)

WebDrake commented 4 years ago

I assume the intent here is to reduce the amount of data that needs to be downloaded to check out the code. This isn't particularly a bottleneck on overall build time but is probably a nice thing to do just for local testing (especially when running containerized builds).

denizzzka commented 4 years ago

but is probably a nice thing to do just for local testing

Yes, sure! LLVM codebase is too huge

As a general rule it's nice if all contributors can write commit messages that make clear both what has changed and why (including what the expected impact will be).

Sorry.

WebDrake commented 4 years ago

Sorry.

No apologies needed -- thanks for being receptive to the feedback.

I would suggest an imperative style title like:

Use source-depth: 1 to speed up git checkouts

And maybe add a small note below explaining the practical meaning of this (i.e. that source-depth limits the amount of historical commit data that is loaded).

Thanks in any case for the contribution, and sorry if the semver branch style is confusing -- I should add some notes to the README with advice for contributors.

BTW, were the git checkout times particularly problematic for you? For me it's a small amount of time compared to the overall build + test time, so it's good to know how much of an impact there is for others.

denizzzka commented 4 years ago

were the git checkout times particularly problematic for you?

No. But without visual representation of branches tree it was difficult to understand what's going on here.

In my opinion, drop of ~master does not give any advantages.

WebDrake commented 3 years ago

Closing as this is no longer needed: the snap package was reworked to use upstream builds.