-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
lint: Do not check *.md
files for white noise
#4662
Conversation
Hm OK, so |
Signed-off-by: Matej Gera <[email protected]>
1388b7f
to
e28d5d4
Compare
*.md
files in lint
target
*.md
files in lint
target*.md
files for white noise
I think the solution here is to drop |
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Thanks for this. Though I'm somewhat curious as my recent PR did have white noise in the docs. Like the change I made actually looks good and it should be done like that? |
@wiardvanrij The issue I tried to solve here is we have two 'competing' targets - both try to format IMHO we should:
|
yea totally agree. Remove the check on |
Signed-off-by: Matej Gera <[email protected]>
Signed-off-by: Matej Gera <[email protected]>
Signed-off-by: Matej Gera <[email protected]>
56a2cbe
to
6034430
Compare
Signed-off-by: Matej Gera <[email protected]>
It looks like ultimately, it's |
ping @wiardvanrij does this look good to you now? |
Yup I think this makes sense! Looks good 👍 |
Makefile
Outdated
@@ -227,7 +227,9 @@ docs: build examples $(MDOX) | |||
check-docs: ## checks docs against discrepancy with flags, links, white noise. | |||
check-docs: build examples $(MDOX) | |||
@echo ">> checking formatting and local/remote links" | |||
PATH=${PATH}:$(GOBIN) $(MDOX) fmt --check -l --links.localize.address-regex="https://thanos.io/.*" --links.validate.config-file=$(MDOX_VALIDATE_CONFIG) $(MD_FILES_TO_FORMAT) | |||
PATH=${PATH}:$(GOBIN) $(MDOX) fmt $(MD_FILES_TO_FORMAT) |
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.
Why did we drop --check -l --links.localize.address-regex="https://thanos.io/.*" --links.validate.config-file=$(MDOX_VALIDATE_CONFIG)
from here? 🤔
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 for catching this, I guess I removed it accidentally during some testing. Fixing it right now. It looks like we'll need to drop the --check
flag still, since otherwise the mdox
command fails (due to diff in white space handling). However, that should be fine, since below we are checking for clean work tree, which would catch any formatting issues.
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.
ping @GiedriusS - hopefully now all is good, the PR might need one more re-run since doc check failed on a link check...
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.
Restarting 👍
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.
Seems like https://blog.taboola.com/category/engineering/monitoring-and-metering-scale/ doesn't exist anymore? Maybe we can delete link to it?
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 found the link it was moved to, made the change, hopefully now it succeeds.
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.
>> detecting white noise
cannot 'run make docs and commit changes': you have unstaged changes.
M docs/proposals-accepted/202108-more-granular-query-performance-metrics.md
mhm ((:
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.
It never ends, does it 😏. Fixed now, the formatting also insisted on replacing _
with *
in CHANGELOG.md
so I added that as well.
Signed-off-by: Matej Gera <[email protected]>
Signed-off-by: Matej Gera <[email protected]>
Head branch was pushed to by a user without write access
Signed-off-by: Matej Gera <[email protected]>
Signed-off-by: Matej Gera <[email protected]>
Head branch was pushed to by a user without write access
Signed-off-by: Matej Gera [email protected]
Changes
make lint
currently clashes withmake check-docs
due to difference in formatting. To resolve this, I suggest to drop*.md
from being checked against white noise in thelint
target, as this should be taken care of incheck-docs
target.Verification
Ran
make lint
andmake check-docs
successfully.