-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Actions menu description #48
Actions menu description #48
Conversation
It then passes an action with this empty description to the template, where it becomes an empty title attribute. plone#24
@kshitiz305 you need to sign the Plone Contributor Agreement to merge this pull request. Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement |
@kshitiz305 thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
@jenkins-plone-org please run jobs |
It should maybe be action.get("description", "") so that the default remains an empty str instead of None, just in case that matters |
Let me raise a new commit for it |
It then passes an action with this empty description to the template, where it becomes an empty title attribute. plone#24
@kshitiz305 you need to sign the Plone Contributor Agreement to merge this pull request. Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement |
It then passes an action with this empty description to the template, where it becomes an empty title attribute. plone#24
@kshitiz305 you need to sign the Plone Contributor Agreement to merge this pull request. Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement |
@jenkins-plone-org please run jobs |
@ewohnlich could you kindly do a code review and suggest any other changes to pass the Jenkin tests. |
I'm afraid I don't know much about the build/jenkins environment here. Maybe @gforcada ? |
@kshitiz305 hi and thank you for your contribution! Did you already sign the contributors agreement here https://plone.org/foundation/contributors-agreement ? If not please do so, otherwise we cannot merge this PR. |
@kshitiz305 you need to sign the Plone Contributor Agreement to merge this pull request. Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement |
@kshitiz305 thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
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.
Thanks!
Re-running the jenkins jobs will probably clear the errors, we had a problem a few weeks ago with plone.app.caching
, but they should be fine now
Co-authored-by: Gil Forcada Codinachs <gil.gnome@gmail.com>
@kshitiz305 you need to sign the Plone Contributor Agreement to merge this pull request. Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement |
@jenkins-plone-org please run jobs |
@kshitiz305 it's already time to clear your contributor agreement, otherwise this PR will never get merged 😓 do you mind having a look at it? 👍🏾 |
Will do it by today itself |
@gforcada I have signed the agreement Please do let me know if there is any thing pending from my end. |
@jenkins-plone-org please run jobs |
@kshitiz305 thanks for signing the contributor agreement! let's see if it is processed quickly 🤞🏾 meanwhile double check your mail, as you will get an invitation to join the Plone organization ✨ Thanks for your efforts! |
@jenkins-plone-org please run jobs |
The requested change to the news snippet has been made.
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.
LGTM, thanks!
The Plone 6.1 Jenkins job fails with a version conflict, but that was already solved, so a rerun should fix it.
I see you are a member of the Plone organisation on GitHub now, welcome! Let me close and reopen this PR to see it that fixes the check for that.
@kshitiz305 thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
The check now says:
Great! The Jenkins 3.8 and 3.11 jobs were passing, so I will just rerun the 6.1 manually. Can be merged when that one is green. |
@jenkins-plone-org please run jobs |
Only one random test failure in plone.app.multilingual, unrelated. I will merge. |
It then passes an action with this empty description to the template, where it becomes an empty title attribute.
Closes #24