-
Notifications
You must be signed in to change notification settings - Fork 86
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
[CI] Added job adding preview links #2561
base: master
Are you sure you want to change the base?
Changes from all commits
4f60f1e
046e515
53947ae
10d9f23
95c79af
ddfbd43
4f62e92
b775b47
3c9440e
0c2b3ff
3b03c8c
63f434d
0e4db96
4167e97
9629530
06547de
62763f9
1ceb979
f457d43
d3b9363
0d25192
35f93ad
4a8fff1
9dd1f40
3cf1b26
18646dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,70 @@ | ||||||
name: "Post preview links for changed files" | ||||||
|
||||||
on: | ||||||
pull_request: | ||||||
paths: | ||||||
- 'docs/**.md' | ||||||
workflow_call: | ||||||
inputs: | ||||||
project: | ||||||
description: 'The project to build (dev doc, user doc)' | ||||||
default: '' | ||||||
required: false | ||||||
type: string | ||||||
|
||||||
jobs: | ||||||
post-preview-links: | ||||||
name: Post preview links for changed files | ||||||
runs-on: ubuntu-latest | ||||||
permissions: | ||||||
# Needed to manage the comment | ||||||
pull-requests: write | ||||||
|
||||||
steps: | ||||||
- uses: actions/checkout@v4 | ||||||
with: | ||||||
fetch-depth: 0 | ||||||
|
||||||
- name: Create comment for changed files | ||||||
run: | | ||||||
file_limit=100 | ||||||
build_url="https://ez-systems-developer-documentation--${{ github.event.pull_request.number }}.com.readthedocs.build/${{inputs.project}}en/${{ github.event.pull_request.number }}/" | ||||||
|
||||||
changed_files=$(git diff --name-only HEAD "origin/$GITHUB_BASE_REF" | grep -E ".md$" || [[ $? == 1 ]]) | ||||||
number_of__changed_files=$(echo "$changed_files" | wc -l) | ||||||
|
||||||
if [[ $changed_files -eq "" ]] ; then | ||||||
comment="Preview of modified files:: no markdown files changed, no preview needed." | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I modify a code sample, a preview is needed. I would reword to stick to the scope.
Suggested change
|
||||||
elif [[ $number_of__changed_files -gt file_limit ]] ; then | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It works without the dollar??
Suggested change
|
||||||
comment="Preview of modified files: too many files modified in a single PR. Unable to post preview links, sorry!" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
else | ||||||
filenames=$(echo "$changed_files" | rev | cut -d / -f 1 | cut -d . -f 2- | rev) | ||||||
|
||||||
# Guess the URL based on Markdown file location. Remove the .md extension. | ||||||
urls=$(echo "$changed_files" | cut -d / -f 2- | rev | cut -d '.' -f 2- | rev | sed -e "s ^ $build_url ") | ||||||
|
||||||
left=$(yes "+ [" | head -n "$number_of__changed_files" ) | ||||||
middle=$(yes "](" | head -n "$number_of__changed_files" ) | ||||||
right=$(yes ")\n" | head -n "$number_of__changed_files" ) | ||||||
|
||||||
comment=$(paste -d'\0' <(echo "$left") <(echo "$filenames") <(echo "$middle") <(echo "$urls") <(echo "$right")) | ||||||
comment="Preview of modified files:\n\n$comment" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
fi | ||||||
|
||||||
echo -e $comment > comment.md | ||||||
|
||||||
- name: Find comment | ||||||
id: find-comment | ||||||
uses: peter-evans/find-comment@v3 | ||||||
with: | ||||||
issue-number: ${{ github.event.pull_request.number }} | ||||||
comment-author: 'github-actions[bot]' | ||||||
body-includes: 'Preview of modified files' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
- name: Create or update comment | ||||||
uses: peter-evans/create-or-update-comment@v4 | ||||||
with: | ||||||
comment-id: ${{ steps.find-comment.outputs.comment-id }} | ||||||
issue-number: ${{ github.event.pull_request.number }} | ||||||
body-path: comment.md | ||||||
edit-mode: replace |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
Attributes: | ||
- Size ([XS, S, M, L, XL]) - a generic one | ||
- Frame Shape/Features | ||
- Material - a generic one | ||
- Paint job/color - would probably require a couple of those (and a "Paints" attribute group) to take into account the various manufacturers and the parts | ||
- | ||
|
||
Hi! | ||
|
||
I went through the PIM mnodeling exercise. | ||
|
||
I watch a couple of math youtube channels and they're sometimes doing what they call "building intuition" - going through a couple of handpicked examples so that the viewer understand what's going on under the hood and gains deeper knowledge of the subject. I think you're doing the same with the Product Types explanation - showing the implementation details so that the readers knows what's going on. Nicely done! | ||
|
||
About the modeling exercise: | ||
- I've approached it in two phases: | ||
1) Modeling attributes | ||
2) Modeling Product Types | ||
|
||
About modelling attributes: | ||
- It feels extremely tempting to model similar attributes as a single attribute. For example to have a single attribute for size: [XS, S, M, L, XL]. | ||
But I think it's a trap - even though both the frame and the fork might be size XL, if at some point their actual dimensions might have to be displayed (like in tshirt, where you often get additional info when buying - chest diameter) then bundling these two would be a problem. | ||
|
||
Same for color - one manufacturer's red might be different from the red from another one. Bundling them together might be a problem (especially when one part comes in 5 colors and there's a million for the second one - it would be confusing for the PIM manager to choose the right one). | ||
|
||
About modelling product types: | ||
- I didn't notice the series at first, so my initial idea was to model each model separately (Product Types: Fuji, Matterhorn, Annapurna, ...). I think it's mostly because the Series is not visible in the table - and started analyzing based on this :D | ||
- Then I noticed the series: and I think the suggested solution makes sense, if the models in each series are similar to each other. | ||
|
||
I started to think how can the product type information be used. Let's assume: | ||
- Series 4 might be targeted for families, and Series 5 might be targeted for semi-professionals | ||
|
||
My initial idea: | ||
- so I'd use the product type to display the right templates etc. | ||
|
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 manage to generate the whole link list in one go:
git diff --name-only HEAD "origin/master" | grep -E "^docs\/.*\.md$" | sed -E "s|^docs/(.*)\.md$|- [docs/\1.md](${build_url}\1/)|"