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

Allow for structured variables on the scheduled product page #5972

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

r-richardson
Copy link
Contributor

@r-richardson r-richardson commented Oct 2, 2024

This PR fixes the scheduled product page loading issues, which occur when job settings contain arrays, mentioned in https://progress.opensuse.org/issues/166154

example:
Screenshot from 2024-10-09 17-14-31

@perlpunk
Copy link
Contributor

perlpunk commented Oct 2, 2024

I found a bug. The display of multiline fields is now broken:
before
scheduled-product-master
after
scheduled-product-PR

@r-richardson r-richardson force-pushed the handle_jobsetting_arrays branch 3 times, most recently from 7fd1a32 to 5be4fba Compare October 8, 2024 13:28
@r-richardson r-richardson changed the title WIP Allow for structured variables on the scheduled product page Allow for structured variables on the scheduled product page Oct 9, 2024
@r-richardson r-richardson marked this pull request as ready for review October 9, 2024 11:40
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

Please fix your commit that still says "WIP"

@Martchus
Copy link
Contributor

Martchus commented Oct 9, 2024

And could you provide a screenshot so we can see how it looks like when one has such "structured variables" on a product?

@r-richardson r-richardson force-pushed the handle_jobsetting_arrays branch 2 times, most recently from 5d46779 to 8c39e72 Compare October 9, 2024 15:20
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

What you put into the description of the pull request could also be in the commit message details. I don't know how you created your pull request but your git commit messages has only the subject line while the pull request description has more details. Please keep in mind that the github pull request description is only visible on github, the commit can be considered permanent information storage.

I can recommend the tool hub (zypper in rubygem-hub) for easier PR creation. Also, I myself use a script git-pr-last to create a PR with proper description for these simple one-commit PRs:

$ cat $(which git-pr-last )
#!/bin/sh -e
target="${target:-"$USER"}"
git push $target && git show --no-patch --format=%B | hub pull-request -F -

You might want to use the more up-to-date tool https://github.com/cli/cli

See https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/CONTRIBUTING.md#coding-style for more details

assets/javascripts/openqa.js Outdated Show resolved Hide resolved
@r-richardson r-richardson force-pushed the handle_jobsetting_arrays branch 2 times, most recently from 3594b25 to fac83e8 Compare October 9, 2024 15:53
assets/javascripts/openqa.js Outdated Show resolved Hide resolved
assets/javascripts/openqa.js Outdated Show resolved Hide resolved
Copy link
Contributor

@b10n1k b10n1k left a comment

Choose a reason for hiding this comment

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

Minor changes requested. they are calisthenic touches and nothing to do with the logic. Just following some good practices. thank you man and let me know if you have objections

@Martchus
Copy link
Contributor

Martchus commented Oct 10, 2024

@b10n1k "calisthenic touches" as in https://en.wikipedia.org/wiki/Calisthenics ? This seems highly unrelated :-)

Copy link
Contributor

@b10n1k b10n1k left a comment

Choose a reason for hiding this comment

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

Sorry but I just noticed that I missed to press the comment button or something in the morning and one of my comments was missing.

assets/javascripts/openqa.js Outdated Show resolved Hide resolved
This commit fixes the scheduled product page loading issues,
which occur when job settings contain arrays.
Related ticket: https://progress.opensuse.org/issues/166154
Copy link

codecov bot commented Oct 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.75%. Comparing base (d621070) to head (7fe6d83).
Report is 48 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5972      +/-   ##
==========================================
- Coverage   98.75%   98.75%   -0.01%     
==========================================
  Files         396      396              
  Lines       38984    38984              
==========================================
- Hits        38500    38497       -3     
- Misses        484      487       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mergify mergify bot merged commit 70b8882 into os-autoinst:master Oct 10, 2024
45 checks passed
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.

5 participants