Skip to content

Comments

XREF purge: last stand#1355

Merged
CyberShadow merged 1 commit intodlang:masterfrom
aG0aep6G:kill-xref
Dec 10, 2016
Merged

XREF purge: last stand#1355
CyberShadow merged 1 commit intodlang:masterfrom
aG0aep6G:kill-xref

Conversation

@aG0aep6G
Copy link
Contributor

@aG0aep6G aG0aep6G commented Jun 8, 2016

Do not merge before 2.072. The stable docs still use the XREF macros. (Good to go now.)

Finally get rid of XREF, XREF_PACK, XREF_PACK_NAMED, FULL_XREF, CXREF, and ECXREF.

There is one last instance of XREF in phobos. dlang/phobos#4418 takes care of it. (PR has since been merged.)
phobos and druntime are squeaky clean.
In dmd, there are only definitions left in its changelog.dd. dlang/dmd#5843 removes them.

I've checked with grep -r XREF which would detect all the variants as they all contain "XREF".

@wilzbach
Copy link
Contributor

wilzbach commented Jun 8, 2016

You are doing a great job, Mister XREF-Purger (aka Mr. XP)

Unfortunately it seems that we have to wait for the next Phobos release, before we can finally kill XREF:

+<div class="keyval Throws"><span class="key keyThrows">Throws:</span> <div class="val valThrows"><!--UNDEFINED MACRO: "XREF"--> if the number exceeds
         the target type's range.</div></div>

maybe you can separate the final kill in another PR, so that we don't forget it?

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Jun 8, 2016

You are doing a great job, Mister XREF-Purger (aka Mr. XP)

Mr XP?

Unfortunately it seems that we have to wait for the next Phobos release, before we can finally kill XREF

Ah, right. Didn't think of the stable docs.

maybe you can separate the final kill in another PR, so that we don't forget it?

Makes sense. I've moved the sed part of this to #1357. This pull request now only removes the definitions. It shouldn't be pulled before the 2.072 release.

@adamdruppe
Copy link
Contributor

Let's remember to write a README on dlang.org explaining the correct
usage of the ref macros too. So it is easier for authors to keep up

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Jun 8, 2016

Let's remember to write a README on dlang.org explaining the correct usage of the ref macros too. So it is easier for authors to keep up

There are explanations in dlang.org.ddoc. Good enough, or what do you have in mind?

@wilzbach
Copy link
Contributor

wilzbach commented Jun 8, 2016

Mr XP

Sorry about the joke ;-)

Good enough, or what do you have in mind?

I agree with @adamdruppe that we should list the most common ddoc macros and tricks in the dlang.org README.
For example most people don't know that they need to escape occurring variables or names in links with _, because otherwise ddoc automatically applies an italic span around them.
Also the dlang.org.ddoc file isn't easy to find if you are new to ddoc/D.

@adamdruppe
Copy link
Contributor

On Wed, Jun 08, 2016 at 09:43:47AM -0700, Sebastian Wilzbach wrote:

For example most people don't know that they need to escape occurring variables or names in links with _, because otherwise ddoc automatically applies an italic span around them.

We can actually fix that and really should by just defining:

DDOC_PSYMBOL=$0

so suppress the wrapping. Then we pretend that misfeature doesn't exist and make everyone's life better instead of littering _ in every other sentence.

@adamdruppe
Copy link
Contributor

On Wed, Jun 08, 2016 at 09:39:00AM -0700, aG0aep6G wrote:

There are explanations in dlang.org.ddoc. Good enough, or what do you have in mind?

I mean the README file itself so it is right in the first place people look.

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Jun 8, 2016

I mean the README file itself so it is right in the first place people look.

Do contributors check that file regularly? I don't.

I agree with @adamdruppe that we should list the most common ddoc macros and tricks in the dlang.org README.
For example most people don't know that they need to escape occurring variables or names in links with _, because otherwise ddoc automatically applies an italic span around them.
Also the dlang.org.ddoc file isn't easy to find if you are new to ddoc/D.

Ok, yeah, makes sense. There's also CONTRIBUTING.md, linked from README.md. Maybe that's the right spot for this?

@adamdruppe
Copy link
Contributor

On Wed, Jun 08, 2016 at 09:54:43AM -0700, aG0aep6G wrote:

Do contributors check that file regularly? I don't.

Regular contributors might not but infrequent contributors or newbies surely would. I don't commit much to dlang.org in part because I so often forget how it works, since I get the urge to only a few times a year.

@wilzbach
Copy link
Contributor

wilzbach commented Jun 8, 2016

Ok, yeah, makes sense. There's also CONTRIBUTING.md, linked from README.md. Maybe that's the right spot for this?

Sounds good - maybe we should also add some pointers from the contributing guide of Phobos (and maybe DRuntime and DMD), so that people can find the guide?

@aG0aep6G aG0aep6G changed the title XREF purge: last stand [Don't merge before 2.072] XREF purge: last stand Jun 8, 2016
@aG0aep6G aG0aep6G force-pushed the kill-xref branch 3 times, most recently from e482466 to c9431ea Compare December 4, 2016 20:54
@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 4, 2016

With 2.072.1 released, all traces of the XREF macros are gone now. This should be good to go, but the doc tester spews 500s. Ping @CyberShadow

@aG0aep6G aG0aep6G changed the title [Don't merge before 2.072] XREF purge: last stand XREF purge: last stand Dec 4, 2016
@CyberShadow
Copy link
Member

Fixed the diffs, thanks for the ping.

@CyberShadow
Copy link
Member

Looks like a few made their way back in?

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 5, 2016

Looks like a few made their way back in?

  1. Phobos stable hasn't been merged into master for a while, so master is currently missing the latest whack-an-XREF-mole commit. PR to merge stable into master: merge stable into master phobos#4932

  2. Looks like the doc tester considers 2.072.0 to be the latest release. Looks like something's going wrong. Should be 2.072.1, right?

@CyberShadow
Copy link
Member

Looks like the doc tester considers 2.072.0 to be the latest release. Looks like something's going wrong. Should be 2.072.1, right?

Fixed, should take effect on the next build.

@aG0aep6G aG0aep6G force-pushed the kill-xref branch 2 times, most recently from a405ff7 to f4412f8 Compare December 6, 2016 12:06
@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 6, 2016

Fixed, should take effect on the next build.

Still shows 2.072.0 after a rebuild via empty amend and push -f. http://dlang.org itself shows 2.072.0, too.

@CyberShadow
Copy link
Member

Whoops, forgot to update submodules. Should be really fixed now. Also fixed the website, double whoops!

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 6, 2016

Should be really fixed now.

The tester now shows diffs like this for everything:

-            switch to <a href="../phobos/index.html">2.072.0</a>. <br>
+            switch to <a href="../phobos/index.html">2.072.1</a>. <br>

It also shows some other stuff that seems unrelated to this PR.

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 6, 2016

Re-pushed once again. Diff is smaller now, but I can't make sense of it. Looks like unrelated changes.

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 7, 2016

Alright. I don't know if you fixed anything, @CyberShadow, but the doc tester says "no changes" now. Seems we're good to go.

@wilzbach
Copy link
Contributor

Alright. I don't know if you fixed anything, @CyberShadow, but the doc tester says "no changes" now. Seems we're good to go.

Awesome! Great job!

@CyberShadow CyberShadow merged commit a63e041 into dlang:master Dec 10, 2016
@CyberShadow
Copy link
Member

Oops, sorry, forgot about this. Merged, thanks!

@aG0aep6G aG0aep6G deleted the kill-xref branch December 10, 2016 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants