N0ury master #514
N0ury master #514
Conversation
|
@niccokunzmann 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! |
Pull Request Test Coverage Report for Build 4682326578
💛 - Coveralls |
|
Interesting ... There are new Jenkins jobs appearing. I can understand the changelog one. I will experiment with the others. |
|
This PR does indeed fix #348. However, I wonder for the tests how the values can be accessed: Is now: for this line: |
|
I fixed the test with the help of the documentation. The current error is that the parameter is missing a character. On it ... |
|
@mauritsvanrees The plone PR tests were added and I do not know if they work for this module. They just appeared. I will run them but I do not know if they actually mean something. Could you check that they use the code from this PR when they run? |
|
@mauritsvanrees The Plone 3.8 test fails. |
|
I see the Jenkins tests pass on Python 3.9, 3.10 and 3.11 and only fail on 3.8. The two failures are in robot framework tests, and there can be some instability in there. Looks unrelated. But really, the Jenkins jobs should not run. We have a GitHub Actions test in this repo that uses Plone, and that should be enough. I think I or someone else has pushed a button to automatically re-enable these tests on lots of repositories. For the record, the Jenkins jobs do use the branch from this PR. See the 3.8 console: So: should be fine to merge and release, but I will leave both to you. Thanks! |
|
@mauritsvanrees Thanks for the clarification! I like the changelog test by the way. That is a handy reminder. If you like to enable this again, that should be ok to try out. |
See collective/icalendar#514 (comment) and following comments. From a Plone core development perspective, `icalendar` is a third party package since a while. It is used a lot outside of Plone, and is taken care of by people who are not core Plone developers. The `plone.app.event` tests are run on every PR of this package, see [yaml file](https://github.com/collective/icalendar/blob/v5.0.5/.github/workflows/tests.yml#L26) and [`tox.ini`](https://github.com/collective/icalendar/blob/v5.0.5/tox.ini#L27). I keep tabs on new versions, and manually update its version in `buildout.coredev`. So nothing is needed on Jenkins, so `mr.roboto` does not need to do anything. I have removed the roboto hooks in the settings of the `icalendar` repo. But: checking for a changelog entry is still wanted. That is where this PR comes in: 1. Add `icalendar` to all ignore lists, except for the required changelog. (Also sort them alphabetically.) 2. Fix the `githubcommithooks` endpoint to look for a single repo in the collective if it is not in Plone. With this in place, I can enable the hooks again for this single repo, and only the changelog entry would be checked.
@niccokunzmann The hooks for that should have been restored just now. Please ping me if anything goes wrong with new PRs. Only the changelog test should be active. |
This updates #502 to have a merge with master.