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

DS-2758 improve broken links workflow #1417

Closed
wants to merge 2 commits into from

Conversation

peterswirl
Copy link
Collaborator

Branching Reminders

Branching off of develop as we will want this in both cases as README.md is included in the scan. Happy to redo if I'm mistaken

Description

Improve workflow - auto run for PR to develop and main

  • fix csv format - urlstechie/[email protected] potentially generates partially quoted-csv
  • generate summary in markup - run decorated with summary
  • on failure:
    • store csv file
    • add message to PR (not really necessary but makes the issue very salient

Related Issue(s)

Internal issue DS-2758

Testing and Validation

Setup on Fork and ran PR with broken doc and that after fix

Broken case

image

After Fix

No comment on PR and the run looks like
image

Type of Change

  • Bug fix or other non-breaking change that addresses an issue
  • New Feature / Enhancement (non-breaking change that add or improves functionality)
  • New Feature (breaking change that is not backwards compatible and/or alters current functionality)
  • Documentation (change to product documentation or README.md only)

NOTE: this is actually more of a process change for non shipping code. So, it could be a new feature, but it is not really for the product but its development processes

- auto run for PR to develop and main
- fix csv format - it's potentially partially quoted-csv
- generate summary in markup
- run decorated with summary
- on failure:
  - store csv file
  - add message to PR (not really necessary but makes the issue very salient
@peterswirl peterswirl self-assigned this Aug 22, 2024
Comment on lines +88 to +95

if failed_count > 0:
summary.write("\n### Failed URLs:\n")
summary.write("* " + "\n* ".join(failed_urls) + "\n")

if unhandled_count > 0:
summary.write("\n### Unhandled Lines:\n")
summary.write("* " + "\n* ".join(unhandled_lines) + "\n")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI, I could include the excluded. Let me know if we want to do that

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @peterswirl ,
Adding the README is a great idea. But do we need to run this on merges to develop branch? We don't maintain the README.md on that branch (we don't take it into releases, for example). And the docs/ directory only exists on main. Maybe one of those things is changing based on what you were told, but that's my understanding at the moment. Good stuff otherwise for sure!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that was my question. So, this is process related but,... docs do go straight to main. Let me redo this using main

@peterswirl peterswirl closed this Aug 23, 2024
@peterswirl peterswirl deleted the feature/DS-2758-broken-links branch August 23, 2024 13:58
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.

2 participants