-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
yaml.markdown: Link configuration.yaml at first usage #29280
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to merge this change, but that's mainly because we don't use text in single quotes as a link.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
I think we do? Looking at this, I think it is smart and could be applied to any occurrence of |
Pretty sure there's a few examples around, and not just what I may have introduced...
Usually I find if it's a question that one newer person has had, it's a question that many others have had. And things you can do to make onboarding better/easier is always a good thing. Can we do this a bit more automatically? Like as a build step, doing a string replace? I suspect there might be a few other similar ones that could be beneficial. Or would it require at least changing all usages/mentions like this into a template/variable or similar? |
We should not. It can also be used in code snippets. |
You mean you don't want to write a backtracking regex to look for whether we're in a code block? ;) |
I mean we should just use Markdown and don't rely on magic. |
Can this be merged then, as is? |
We are awaiting @klaasnicolaas, he left this review. Please have patience, as most people do this in their spare time. ../Frenck |
I indeed see that we are applying it on more places, but that is more sporadic. Of course it is a good improvement, but perhaps we need to be consistent in how we handle links and highlighted text... it can be confusing. But otherwise this PR is good to go for merging. |
Alright, so agreed we can move forward I guess. That said, this change is invalid nevertheless, as it links to itself 😉 ../Frenck |
Copy paste is hard (sometimes)! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @reedy 👍
../Frenck
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @reedy 👍
../Frenck
I'll have a bit of a think about proliferating this a bit more throughout the other doc pages. Though, maybe as I'm making changes to other docs for other reasons, I can include a similar change to this too. A very quick search suggests there's over 2000 mentions of the string |
Proposed change
Link to configuration.yaml's documentation at first usage
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.