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

Enable markdownlint check in CI #2773

Merged
merged 6 commits into from
Oct 15, 2020
Merged

Enable markdownlint check in CI #2773

merged 6 commits into from
Oct 15, 2020

Conversation

roji
Copy link
Member

@roji roji commented Oct 15, 2020

Same as how it's done in dotnet/docs - thanks @nschonni @mairaw! This adds an additional check for linting. IIRC we don't currently block merging on failed checks in this repo, so we can treat this as informative warnings if we want (though it's a good idea to start paying attention to it and keeping things clean).

Also cleaned up existing issues.

Closes #1852

And clean up to get to green

Closes #1852
@roji roji marked this pull request as ready for review October 15, 2020 10:25
@@ -0,0 +1,21 @@
name: Markdownlint

on: pull_request
Copy link
Contributor

@nschonni nschonni Oct 15, 2020

Choose a reason for hiding this comment

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

You can filter this with the "paths" so it only runs if affected files are touched. The latest version has an example of that https://github.com/dotnet/docs/blob/ab63ef519c0cd889fffb6b243a4bb85720eb5d29/.github/workflows/markdownlint.yml

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it worth it, considering that we tell markdownlint what to exclude anyway below? If this would be just about not running the check at all for EF6 changes...

.github/workflows/markdownlint.yml Outdated Show resolved Hide resolved
@roji roji requested a review from smitpatel October 15, 2020 14:52
@@ -188,7 +188,7 @@ You can use the string overload of `HasForeignKey(...)` to configure a shadow pr

#### Foreign key constraint name

By convention, when targeting a relational database, foreign key constraints are named FK_<dependent type name>_<principal type name>_<foreign key property name>. For composite foreign keys <foreign key property name> becomes an underscore separated list of foreign key property names.
By convention, when targeting a relational database, foreign key constraints are named FK\_\<dependent type name>\_\<principal type name>\_\<foreign key property name>`. For composite foreign keys, \<foreign key property name> becomes an underscore separated list of foreign key property names.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By convention, when targeting a relational database, foreign key constraints are named FK\_\<dependent type name>\_\<principal type name>\_\<foreign key property name>`. For composite foreign keys, \<foreign key property name> becomes an underscore separated list of foreign key property names.
By convention, when targeting a relational database, foreign key constraints are named FK\_\<dependent type name>\_\<principal type name>\_\<foreign key property name>. For composite foreign keys, \<foreign key property name> becomes an underscore separated list of foreign key property names.

@roji roji merged commit 5b9b927 into master Oct 15, 2020
@smitpatel
Copy link
Member

Noooo! 😢

@smitpatel smitpatel deleted the markdownlint branch October 15, 2020 15:05
@roji
Copy link
Member Author

roji commented Oct 15, 2020

What? What did I do?

@smitpatel
Copy link
Member

image

@roji
Copy link
Member Author

roji commented Oct 15, 2020

I've exercised executive powers and pushed a commit to fix the world.

@nschonni
Copy link
Contributor

@roji might take a look later, but FYI, you can now auto-fix some rules in markdownlint. They're listed as "Fixable" in https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md

  1. Install the cli globally
  2. Remove the supressed rule. EX: MD027
  3. Run markdownlint --fix "**/*.md"

@roji
Copy link
Member Author

roji commented Oct 15, 2020

@nschonni oh nice, didn't know that. I think that for our remaining suppressions, some decision-making is typically required - but I'll give it a try.

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.

Integrate markdownlint into our CI pipeline
3 participants