Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented May 12, 2018

No description provided.

@ghost ghost requested review from JackStouffer and burner as code owners May 12, 2018 18:42
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @bbasile! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + phobos#6507"

@ghost
Copy link
Author

ghost commented May 12, 2018

@wilzbach
Copy link
Contributor

IIRC pegged is no longer in development, so I am not sure whether this notification will help :/

@ghost
Copy link
Author

ghost commented May 14, 2018

IIRC pegged is no longer in development, so I am not sure whether this notification will help :/

No it's okay. It's still maintained and the fix is actually in the trunk since a few hours. The author just has to push a git tag and this PR will be green.

Copy link
Member

@jmdavis jmdavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but these should not be removed yet. When it says in the documentation that they're to be removed in May 2018, that means that they're to be removed from the documentation in May 2018. The public documentation needs to be removed and @@@DEPRECATED-2018-05@@@ should be changed to

// Explicitly undocumented. It will be removed in May 2019. @@@DEPRECATED_2019-05@@@

e.g. https://github.com/dlang/phobos/blob/master/std/datetime/timezone.d#L165

The symbols themselves should not be removed yet.

@ghost
Copy link
Author

ghost commented May 15, 2018

Sorry

No problem i didn't know it works like that. Just one think: the problem is that it leads to undocumented public symbols which our linter doesn't like much.

@ghost ghost closed this May 15, 2018
@jmdavis
Copy link
Member

jmdavis commented May 15, 2018

No problem i didn't know it works like that.

It's how it's been working, though there's been enough confusion over it that I'm probably going to need to change how that's handled so that the documentation indicates when the symbol is going to be completely removed, and then there can be a non-public comment indicating when the documentation is supposed to be removed. But I haven't switched over to that yet, and this was deprecated using the approach that we've been using for several years now. So, what's listed in the documentation is the date for removal from the docs, not the final removal date.

the problem is that it leads to undocumented public symbols which our linter doesn't like much.

Are you talking about something in the circleci stuff? Well, it's going to need to be able to handle undocumented, deprecated symbols, because that's a standard part of the deprecation process. Symbols are deprecated and documented for one year, then deprecated and undocumented for a year, and then finally removed. So, they're going to sit there with no public documentation for about a year as a normal part of the process.

@ghost ghost deleted the dep-may-2018 branch May 15, 2018 08:46
This pull request was closed.
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.

5 participants