Re: The case for small diffs in Pull Requests
On Thursday, 21 July 2016 at 03:30:34 UTC, Walter Bright wrote: I've looked at many PRs that consisted of multiple commits. The trouble with them is: 1. they often have nothing in particular to do with each other 2. I may want to pull a subset of the commits, but the only option I have is all or nothing As far as I'm aware git offers the option of cherry picking commits. It will not mark the PR as merged or generally not be what you are looking for, but maybe it's a usable workaround :)
Re: The case for small diffs in Pull Requests
I've looked at many PRs that consisted of multiple commits. The trouble with them is: 1. they often have nothing in particular to do with each other 2. I may want to pull a subset of the commits, but the only option I have is all or nothing
Re: Vision for the D language - stabilizing complexity?
On Wednesday, 20 July 2016 at 20:12:14 UTC, Jack Stouffer wrote: On Tuesday, 19 July 2016 at 15:22:19 UTC, Andrew Godfrey wrote: [...] Something being dfix-able is not enough for the simple reason that legacy code in D is already becoming a thing, despite D2 only existing for nine years. A complaint has arisen many times in the forum and in DConfs that there are many packages on code.dlang.org or on Github that don't compile because the author stopped maintaining them. In many cases, these repo's have only been unmaintained for a year(!) or less, and they already don't compile. [...] Thanks for the explanation. If most changes aren't dfixable (or aren't believed to be), that explains why the discussions I've read don't mention the dfix approach.
Re: Vision for the D language - stabilizing complexity?
On Tuesday, 19 July 2016 at 15:22:19 UTC, Andrew Godfrey wrote: Just like earlier in this thread, where I mentined dfixable breaking changes and Walter implied that even though a would cause people to have to manually rewrite. Something being dfix-able is not enough for the simple reason that legacy code in D is already becoming a thing, despite D2 only existing for nine years. A complaint has arisen many times in the forum and in DConfs that there are many packages on code.dlang.org or on Github that don't compile because the author stopped maintaining them. In many cases, these repo's have only been unmaintained for a year(!) or less, and they already don't compile. There's no way for anyone other than the original author that can fix this; all we can do is add a warning on code.dlang.org. All the while it reduces the signal to noise ratio of good to bad D code online. Every breakage needs to take into account the baggage of visible old code. There's also the point that there are few changes which can be dfix-able (according to its author).
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 19:11:56 UTC, deadalnix wrote: Implementation problem should not be "fixed" by changing the language. I concur. If the root problem is slow compilation, then there are much simpler, non-breaking changes that can be made to fix that.
Re: Phobos doc anchors not friendly for keyboard-centric browsers
On Tue, 19 Jul 2016 14:32:28 +, rcorre wrote: > It's a small thing and I'm probably in a minority who work like this, Things that are difficult with your keyboard-mode browser are probably difficult for people who use screen readers. It's definitely worth bringing up.
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 19:11:56 UTC, deadalnix wrote: Implementation problem should not be "fixed" by changing the language. this, i believe, closes the topic altogether.
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 18:51:49 UTC, Timothee Cour wrote: That means that a change in any single dependency would trigger recompilations in many files. so what? can you even imagine how many things you'll have to recompile if you'll change something in /usr/include? it's just your tools usually ignoring files there, but here you clearly included all system dependencies, so not ignoring libc system include files is a valid point from my side.
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 07:45:12 UTC, Timothee Cour wrote: currently, top-level imports in a module A are visible by other modules B importing A, and are visited (recursively) during compilation of A, slowing down compilation and increasing dependencies (eg with separate compilation model, a single file change will trigger a lot of recompilations). That is purely an implementation problem. SDC doesn't have this problem for instance as it only parse/analyze import on demand. modules imported by A are already not visible to B, but still required sometime to compile B to resolve A's signatures. Implementation problem should not be "fixed" by changing the language.
Re: proposal: private module-level import for faster compilation
Same answer : http://forum.dlang.org/post/nmngk8$inm$1...@digitalmars.com
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 18:21:46 UTC, ketmar wrote:
p.s. the sole improvement in symbol lookup mechanics can speed
up the things by several factors, and without breaking the
language. current dmdfe symbol/template lookup mechanics is not
really fast.
If this example weren't enough, here's the other even more
compelling argument: speedup up recompilation for makefile-like
tools that trigger recompilation when a dependency is modified:
---
module fun1;
import fun2;
void test1(){}
---
module fun2;
version(A) import std.datetime;
//version(proposed_feature) private import std.datetime;
void test2(){}
---
dmd -c -o- -version=proposed_feature -deps fun1.d
would show following dependencies:
fun2
dmd -c -o- -version=A -deps fun1.d
shows following 68 dependencies (68!)
That means that a change in any single dependency would trigger
recompilations in many files.
fun2 core.attribute core.bitop core.exception
core.internal.string core.internal.traits core.memory
core.stdc.config core.stdc.errno core.stdc.inttypes
core.stdc.signal core.stdc.stdarg core.stdc.stddef
core.stdc.stdint core.stdc.stdio core.stdc.stdlib
core.stdc.string core.stdc.time core.stdc.wchar_
core.sys.osx.mach.kern_return core.sys.posix.config
core.sys.posix.dirent core.sys.posix.fcntl
core.sys.posix.inttypes core.sys.posix.signal
core.sys.posix.stdlib core.sys.posix.sys.select
core.sys.posix.sys.stat core.sys.posix.sys.time
core.sys.posix.sys.types core.sys.posix.sys.wait
core.sys.posix.time core.sys.posix.unistd core.sys.posix.utime
core.time core.vararg object std.algorithm
std.algorithm.comparison std.algorithm.iteration
std.algorithm.mutation std.algorithm.searching
std.algorithm.setops std.algorithm.sorting std.array std.ascii
std.bitmanip std.conv std.datetime std.exception std.file
std.format std.functional std.internal.cstring
std.internal.unicode_tables std.meta std.path std.range
std.range.interfaces std.range.primitives std.stdio std.stdiobase
std.string std.system std.traits std.typecons std.typetuple
std.uni
Re: proposal: private module-level import for faster compilation
p.s. the sole improvement in symbol lookup mechanics can speed up the things by several factors, and without breaking the language. current dmdfe symbol/template lookup mechanics is not really fast.
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 18:09:06 UTC, Timothee Cour wrote: this simple example shows this feature would provide a 16X speedup. 100 ms speedup in exchange of creating another special case in language? no, thank you, won't buy. that was exactly what i meant: if we'll look at *real* numbers instead of scales, we'll find that all amazing "speedups" are measured in terms of milliseconds for most projects, and in terms of seconds for 100mb+ projects. breaking language orthogonality for this is something i can't see as improvement. sorry.
Re: proposal: private module-level import for faster compilation
this simple example shows this feature would provide a 16X
speedup.
time dmd -c -o- -version=A -I$code main.d
0.16s
time dmd -c -o- -version=B -I$code main.d
0.01s
---main.d:
module tests.private_import.main;
import tests.private_import.fun;
void test(){}
---
---fun.d:
module tests.private_import.fun;
version(A) import std.datetime;
//version(C) private import std.datetime;
void foo(){
// same as version(C) if this feature were implemented
version(B) import std.datetime;
}
---
Re: The case for small diffs in Pull Requests
On 7/20/16 1:09 PM, Vladimir Panteleev wrote: On Tuesday, 19 July 2016 at 06:29:05 UTC, Jacob Carlborg wrote: On 2016-07-19 00:30, Walter Bright wrote: https://medium.com/@kurtisnusbaum/large-diffs-are-hurting-your-ability-to-ship-e0b2b41e8acf#.h3eo1yvqv I've been advocating for a while now that PRs should be small, incremental, encapsulated and focused. This has not been without controversy. I hope the referenced article is a bit more eloquent and convincing than I have been. I fully agree, the problem is if unfinished features are merged to master, which has happened quite a lot in D. Have you read the solution, linked at the bottom? [1]. As far as I can remember, I have not seen this used in the D projects at all. [1] http://graysonkoonce.com/stacked-pull-requests-keeping-github-diffs-small/ Requiring that all contributors do this would kill D development. This method strikes me as nothing but a very ugly work-around for GitHub not having good facilities of reviewing PRs commit-by-commit. Huh? This works fine. I just used it as an advantage for reviewing: https://github.com/dlang/druntime/pull/1602 There is another, much simpler and IMO better way: [snip] These are all good guidelines! I'm not a big fan of rebasing unless you are testing things out. For example, if you introduce a bug in your PR and then fix it, there's no reason to keep that bug somewhere in history. The idea of squashing commits just because they are too many, I don't see the point. -Steve
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 17:05:11 UTC, Jack Stouffer wrote: IIRC compiler also spends extra work on templates because it doesn't cache the result, so things like isInputRange!(string) could be evaluated hundreds of times for your program. it does cache that (see template merging), it even causing some bugs. yet it is using linear search to find something in cache.
Re: The case for small diffs in Pull Requests
On Tuesday, 19 July 2016 at 06:29:05 UTC, Jacob Carlborg wrote: On 2016-07-19 00:30, Walter Bright wrote: https://medium.com/@kurtisnusbaum/large-diffs-are-hurting-your-ability-to-ship-e0b2b41e8acf#.h3eo1yvqv I've been advocating for a while now that PRs should be small, incremental, encapsulated and focused. This has not been without controversy. I hope the referenced article is a bit more eloquent and convincing than I have been. I fully agree, the problem is if unfinished features are merged to master, which has happened quite a lot in D. Have you read the solution, linked at the bottom? [1]. As far as I can remember, I have not seen this used in the D projects at all. [1] http://graysonkoonce.com/stacked-pull-requests-keeping-github-diffs-small/ Requiring that all contributors do this would kill D development. This method strikes me as nothing but a very ugly work-around for GitHub not having good facilities of reviewing PRs commit-by-commit. There is another, much simpler and IMO better way: 1. Contributors: Split the change into commits. Each commit should represent an atomic change, and all commits should compile and pass tests. (See also Linux kernel code guidelines.) 2. Reviewers: For multi-commit PRs, ALWAYS look at the changes one commit at a time. Read the commit messages. Don't even think of clicking on the "Diff" tab, all this does is cause threads like these to appear. (Seriously people, why is this an issue?? I think that these threads will never stop until someone hacks the "Diff" tab out of Walter's GitHub UI.) 3. Contributors: Don't rebase your PR until it's ready to merge! Rebasing will a) erase comments left on individual commits b) make it difficult for reviewers to see new changes since their last review. 4. Reviewers: DO use the new GitHub feature which shows a diff of changes since your last visit. DON'T ask contributors to rebase their PRs until it's ready to merge. 5. Contributors: Once the change is approved, optionally rebase the PR and fold the fixup changes into whatever commits they belong in. This has the advantages that: 1. Until the final rebase, you can track the development history of the PR. All feedback and changes in response to it will still be there. 2. It's much easier to review a newer version of a PR, as you can get a diff from the last version you reviewed. 3. The total time to merge the entire change set is going to be much smaller, because there will be one review per PR instead of one review per commit. From personal experience I can affirm that when a change implemented in a few hours or days drags on its review for a few months (or years!) is very fatiguing. Obviously there are some advantages to splitting changes into multiple PRs, such as better UI on GitHub and letting the auto-tester ensure that nothing breaks each step along the way. However, especially considering how long it already takes for even trivial PRs to get merged, I think that a proposal to split changes into multiple PRs would all-in-all be worse for D development.
Re: proposal: private module-level import for faster compilation
On Wednesday, 20 July 2016 at 07:45:12 UTC, Timothee Cour wrote: ... This, and function local imports, are hacks around the actual problem: the compilers spending time on codegen on things your program will never use. IIRC compiler also spends extra work on templates because it doesn't cache the result, so things like isInputRange!(string) could be evaluated hundreds of times for your program. Fixing those two things are the actual solution.
Re: The case for small diffs in Pull Requests
On Wed, Jul 20, 2016 at 04:39:33PM +, Vladimir Panteleev via Digitalmars-d wrote: > On Tuesday, 19 July 2016 at 08:08:21 UTC, qznc wrote: > > I would like to see squashed commits in D. History looks polluted by > > merge commits to me. This is useless for single-commit pull requests > > at least. > > Without merge commits you can't easily trace a commit back to its pull > request, which contains the feedback, review, and often a > significantly better description of the change. Yes, many a time while git bisecting, I've found that merge commits are invaluable in providing a "paper trail" of exactly what happened with the code that caused the problem, even if the PR in question is only a single commit. Without the github PR number, it would be very hard to reconstruct the history of exactly what went on behind the change -- any discussions, rationales, etc.. T -- Без труда не выловишь и рыбку из пруда.
Re: proposal: private module-level import for faster compilation
i can't see what problem this thing is trying to solve. did you ever measured time taken by building AST of imported module? use separate compilation and/or templates to avoid codegen. problem solved.
Re: The case for small diffs in Pull Requests
On Tuesday, 19 July 2016 at 08:08:21 UTC, qznc wrote: I would like to see squashed commits in D. History looks polluted by merge commits to me. This is useless for single-commit pull requests at least. Without merge commits you can't easily trace a commit back to its pull request, which contains the feedback, review, and often a significantly better description of the change.
Re: proposal: private module-level import for faster compilation
I think this is a wrong approach patching a problem instead of fixing it. Real solution would be to improve and mature .di header generation and usage by compilers so that it can become the default way to import packages/libraries.
Re: proposal: private module-level import for faster compilation
typo:
s/dmd -c -o- -deps A.d/dmd -c -o- -deps B.d
On Wed, Jul 20, 2016 at 12:45 AM, Timothee Cour
wrote:
> currently, top-level imports in a module A are visible by other modules B
> importing A, and are visited (recursively) during compilation of A, slowing
> down compilation and increasing dependencies (eg with separate compilation
> model, a single file change will trigger a lot of recompilations).
>
> I propose a private import [1] to mean an import that's only used inside
> function definitions, not on the outside scope. It behaves exactly as if it
> the import occurred inside each scope (function and template definitions).
> This is applicable for the common use case where an import is only used for
> symbols inside functions, not for types in function signature.
>
>
> module A;
> private import util;
> void fun1(){
> // as if we had 'import util;'
> }
>
> void fun2(){
> // as if we had 'import util;'
> }
>
> // ERROR: we need 'import util' to use baz in function declaration
> void fun3(baz a){}
>
>
> module util;
> void bar(){}
> struct baz{}
>
> module B;
> import A;
>
>
> The following should not list 'util' as a dependency of B, since it's a
> 'private import'
> dmd -c -o- -deps A.d
>
>
> The benefits: faster compilation and recompilation (less dependencies).
>
> NOTE [1] on syntax: currently private import just means import, we could
> use a different name if needed, but the particular syntax to use is a
> separate discussion.
>
proposal: private module-level import for faster compilation
currently, top-level imports in a module A are visible by other modules B
importing A, and are visited (recursively) during compilation of A, slowing
down compilation and increasing dependencies (eg with separate compilation
model, a single file change will trigger a lot of recompilations).
I propose a private import [1] to mean an import that's only used inside
function definitions, not on the outside scope. It behaves exactly as if it
the import occurred inside each scope (function and template definitions).
This is applicable for the common use case where an import is only used for
symbols inside functions, not for types in function signature.
module A;
private import util;
void fun1(){
// as if we had 'import util;'
}
void fun2(){
// as if we had 'import util;'
}
// ERROR: we need 'import util' to use baz in function declaration
void fun3(baz a){}
module util;
void bar(){}
struct baz{}
module B;
import A;
The following should not list 'util' as a dependency of B, since it's a
'private import'
dmd -c -o- -deps A.d
The benefits: faster compilation and recompilation (less dependencies).
NOTE [1] on syntax: currently private import just means import, we could
use a different name if needed, but the particular syntax to use is a
separate discussion.
