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

chore: improve markdownlint #946

Merged
merged 12 commits into from
Mar 6, 2023

Conversation

aepfli
Copy link
Member

@aepfli aepfli commented Mar 1, 2023

Markdownlint is a crucial tool for us, but the usability was not as good as it could be.

  1. It was confusing, as it was not accessible within the docs
  2. The configuration deviated from the defaults, IDE-plugins did not pick up those settings
  3. documentation of the setting was missing

With this pull-request we address the points above, and make it easier to be used for our contributors.

Markdownlint is a crucial tool for us, but the usability was not
as good as it could be.

1. It was confusing, as it was not accessible within the docs
2. The configuration deviated from the defaults, IDE-plugins did
   not pick up those settings
3. documentation of the setting was missing

With this pull-request we address the points above, and make it
easier to be used for our contributors.

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 1, 2023
CONTRIBUTING.md Outdated Show resolved Hide resolved
aepfli and others added 2 commits March 1, 2023 11:19
Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Signed-off-by: Simon Schrottner <simon.schrottner@gmail.com>
Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Signed-off-by: Simon Schrottner <simon.schrottner@gmail.com>
CONTRIBUTING.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #946 (16472f1) into main (ef35387) will increase coverage by 0.04%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #946      +/-   ##
==========================================
+ Coverage   57.45%   57.50%   +0.04%     
==========================================
  Files         119      119              
  Lines        9913     9914       +1     
==========================================
+ Hits         5696     5701       +5     
+ Misses       4018     4015       -3     
+ Partials      199      198       -1     
Impacted Files Coverage Δ
metrics-operator/pkg/metrics/server.go 68.80% <0.00%> (+0.25%) ⬆️
...lers/lifecycle/keptnworkloadinstance/controller.go 82.95% <0.00%> (+1.79%) ⬆️
Flag Coverage Δ
certificate-operator 64.15% <ø> (?)
component-tests 50.99% <ø> (+0.66%) ⬆️
klt-cert-manager ?
lifecycle-operator 53.63% <ø> (?)
metrics-operator 62.98% <ø> (+0.03%) ⬆️
scheduler 21.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
StackScribe
StackScribe previously approved these changes Mar 1, 2023
Copy link
Contributor

@StackScribe StackScribe left a comment

Choose a reason for hiding this comment

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

Good stuff -- thanks for writing this up! I added some suggestions for your consideration.

Long term, we should move this content to the Contributing guide but let's commit this as is for now.

Also need to link to the info about markdown-lint from the documentation Contributing.md file (see #758 ).

aepfli and others added 3 commits March 1, 2023 11:51
Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Signed-off-by: Simon Schrottner <simon.schrottner@gmail.com>
Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Signed-off-by: Simon Schrottner <simon.schrottner@gmail.com>
Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Signed-off-by: Simon Schrottner <simon.schrottner@gmail.com>
@aepfli
Copy link
Member Author

aepfli commented Mar 1, 2023

Please do not merge before #866 to reduce merge conflicts :)

thisthat
thisthat previously approved these changes Mar 1, 2023
.markdownlintignore Outdated Show resolved Hide resolved
Copy link
Member

@mowies mowies left a comment

Choose a reason for hiding this comment

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

LGTM generally, but there is already some small documentation in the docs/README.md
I suggest to remove that if we have it in the contributing file

thschue
thschue previously approved these changes Mar 2, 2023
Copy link
Contributor

@thschue thschue left a comment

Choose a reason for hiding this comment

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

lgtm

@StackScribe
Copy link
Contributor

@mowies This file has more details than what is in the CONTRIBUTING.md file and covers software facilities as well as the docs build engine. I am thinking that eventually (not real long term) we should make this doc part of the Contributing Guide and then have the CONTRIBUTING.md files point to that guide for info

@aepfli aepfli dismissed stale reviews from thschue and thisthat via cbbbbaa March 2, 2023 10:01
Co-authored-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Simon Schrottner <simon.schrottner@gmail.com>
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@netlify
Copy link

netlify bot commented Mar 2, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 16472f1
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/640598af051e2c000867eab9
😎 Deploy Preview https://deploy-preview-946--keptn-lifecycle-toolkit.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

mowies
mowies previously approved these changes Mar 2, 2023
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli force-pushed the chore/markdownlint-improvements branch from 7234ce7 to b0dedde Compare March 3, 2023 08:56
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli requested a review from thschue March 3, 2023 13:04
thschue
thschue previously approved these changes Mar 6, 2023
Copy link
Contributor

@thschue thschue left a comment

Choose a reason for hiding this comment

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

lgtm

docs/Makefile Outdated Show resolved Hide resolved
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@thschue thschue left a comment

Choose a reason for hiding this comment

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

lgtm

@thschue thschue merged commit d5d1675 into keptn:main Mar 6, 2023
@keptn-bot keptn-bot mentioned this pull request Mar 6, 2023
thschue pushed a commit to thschue/lifecycle-controller that referenced this pull request Mar 7, 2023
Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Co-authored-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@keptn-bot keptn-bot mentioned this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants