Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing more links #9154

Merged
merged 4 commits into from
Nov 6, 2021
Merged

Fixing more links #9154

merged 4 commits into from
Nov 6, 2021

Conversation

ThomasLandauer
Copy link
Contributor

The first two I missed in #9151
The third is probably older.
Shouldn't the chapter name be displayed as link text by default?? Are you sure that everything is set up correctly with the parser?

The first two I missed in doctrine#9151
The third is probably older.
Shouldn't the chapter name be displayed as link text by default?? Are you sure that everything is set up correctly with the parser?
@ThomasLandauer
Copy link
Contributor Author

@SenseException I just wanted to start changing the order in the table (what I announced in #9131), but the overall structure of the page seems flawed:
Load ClassMetadata Event is a top (!) level heading and "SchemaTool Events" too. And only the former events are included in the table, the latter aren't. I see two ways to fix this:

  1. Include all events in the table. Merge all text starting at Implementing Event Listeners into one section (called something like "List of Events").
  2. Remove those "special" events (onClassMetadataNotFound, postGenerateSchemaTable, etc.) from the table, and leave their text sections where they are. But then move the table closer to "Implementing Event Listeners".

I have no preference, since I still haven't fully understood how all those topics belong together - that's why I started the "overview/table project" ;-)

@SenseException
Copy link
Member

@ThomasLandauer Thanks again. Could you please combine your link-PRs into one PR instead of creating multiple ones? Maybe you could use this one and close the others.

@ThomasLandauer
Copy link
Contributor Author

Just double-checked: They all have specific comments/questions :-(
I usually don't mix unrelated stuff (learned that from symfony-docs ;-) cause when one is problematic, at least the other can be merged in the meantime...
But I'll try to improve in the future :-)

@derrabus derrabus added this to the 2.10.3 milestone Oct 30, 2021
@SenseException
Copy link
Member

I don't want to have multiple commits with the same type of additions. In case of the links we can have them in one PR.

@ThomasLandauer
Copy link
Contributor Author

OK. I integrated two, and closed one.

@SenseException SenseException merged commit a6b7569 into doctrine:2.10.x Nov 6, 2021
@ThomasLandauer ThomasLandauer deleted the patch-1 branch November 6, 2021 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants