Skip to content

Revert "Implement issue# 13996. Add File.tempFile."#3273

Merged
andralex merged 1 commit intomasterfrom
revert-2956-tempFile
May 19, 2015
Merged

Revert "Implement issue# 13996. Add File.tempFile."#3273
andralex merged 1 commit intomasterfrom
revert-2956-tempFile

Conversation

@WalterBright
Copy link
Member

@MartinNowak
Copy link
Member

So what's the lesson here? We can't add a function because it increases the binary size? Ridiculous, let's please fix the compiler instead.

@MartinNowak
Copy link
Member

@WalterBright
Copy link
Member Author

The issue is this function cannot be added until whatever reason it bloats the executable size is fixed. I.e. regressions should not be pulled, regardless of cause.

@andralex
Copy link
Member

Auto-merge toggled on

andralex added a commit that referenced this pull request May 19, 2015
…File

Revert "Implement issue# 13996. Add File.tempFile."
@andralex andralex merged commit c8c5a32 into master May 19, 2015
@WalterBright
Copy link
Member Author

This is why checks for executable size bloat should be in the test suite.

@andralex
Copy link
Member

@MartinNowak do we have that check around yet? I recall you mentioned it at some point.

@MartinNowak MartinNowak deleted the revert-2956-tempFile branch May 20, 2015 11:50
@MartinNowak
Copy link
Member

Yes, here https://github.com/D-Programming-Language/dmd/blob/82b031c22f18334a7edeff437260b6802070a9e3/test/runnable/test13117.d.
Someone should update the thresholds. We can lower the tolerance to maybe 1%, it cannot be zero because the would affect too many pull requests.

@MartinNowak
Copy link
Member

The issue is this function cannot be added until whatever reason it bloats the executable size is fixed.

Sure, but the right direction to fix bugs is going forward not backward.
It was a huge effort to get this functionality into phobos, and not it's reverted because of compiler issues the phobos devs aren't accountable for.

@WalterBright
Copy link
Member Author

I'm sorry about that, but we should never pull something that causes regressions, even if it is not the fault of the new code. This is not about assigning blame.

@schveiguy
Copy link
Member

Can we close the regression now? It's still open.

@andralex
Copy link
Member

@MartinNowak instead of having a tolerance the better policy is to update it on a case basis where justified. @schveiguy yes, could you please close - thanks.

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.

4 participants