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

Update dependencies #95

Merged
merged 6 commits into from
Apr 27, 2022
Merged

Update dependencies #95

merged 6 commits into from
Apr 27, 2022

Conversation

gesellix
Copy link
Contributor

  • bumps the Gradle Docker plugin dependency (along with transitive dependency upgrades, see the release notes)
  • ensures a valid default image name, otherwise tasks might fail (Docker accepts only lowercase image names)

sgrell
sgrell previously approved these changes Apr 25, 2022
Copy link
Contributor

@sgrell sgrell left a comment

Choose a reason for hiding this comment

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

👋 muss man dafür dann ein neues Release machen?

@gesellix
Copy link
Contributor Author

Ja, ich würde angesichts der seit dem letzten Release angesammelten Änderungen ein neues Release empfehlen, ist aber nicht zwingend notwendig. Kommt ein bisschen drauf an, wie aktuell die jeweiligen Buildscripte sind, ich habe diese Version nicht mit Gradle 5 oder 6 getestet, gehe aber davon aus, dass die Versionen weiterhin kompatibel sind.

Vorab, ohne einen konkreten Zeitrahmen zu haben: in der nächsten Version des Gradle -Docker-Plugins werde ich die Kotlin-Version auf mindestens 1.6 anheben, somit auch eine Mindestversion von Gradle voraussetzen. Jetzt also schon mal Vorbereitungen zu treffen kann nicht schaden :)

@sgrell
Copy link
Contributor

sgrell commented Apr 25, 2022

alles klar, danke 😄

@sgrell
Copy link
Contributor

sgrell commented Apr 25, 2022

@gesellix siehe https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflows-in-forked-repositories bedeutet das, dass wir bei uns noch pull_request als Trigger eintragen müssen?

@gesellix
Copy link
Contributor Author

@sgrell stimmt, ja 👍
Es gibt noch pull_request_target als Alternative, um den Umgang mit Secrets zu erleichtern. Siehe auch https://securitylab.github.com/research/github-actions-preventing-pwn-requests/.
Ich denke pull_request reicht hier aus.

@sgrell
Copy link
Contributor

sgrell commented Apr 25, 2022

@gesellix macht das so Sinn? #96

@sgrell sgrell mentioned this pull request Apr 25, 2022
@gesellix
Copy link
Contributor Author

@sgrell ja, ich denke das passt so 👍

@sgrell
Copy link
Contributor

sgrell commented Apr 26, 2022

@gesellix so CI läuft jetzt, ist aber rot, liegt das evtl. an den entfernten envs? Oder daran, dass in dein Repo irgendwas nicht zurückgepusht werden kann?

@gesellix
Copy link
Contributor Author

gesellix commented Apr 26, 2022

@sgrell Wenn ich das richtig sehe, dann ist der Build an sich erfolgreich. Es schlägt das Publishing des Testreports fehl, was wiederum daran liegen kann, dass dem Workflow wegen des Forks das Secret fehlt bzw. nicht die notwendigen Auth-Scopes gesetzt sind. Ich sehe leider keine Details im fehlschlagenden Step, normalerweise taucht sowas auf wie "resource not accessible by integration". Siehst du evtl. mehr Details im Log?

Edit: jetzt sehe ich doch Details, genau wie vermutet, siehe https://github.com/europace/docker-publish-gradle-plugin/runs/6180028906?check_suite_focus=true#step:5:14

Siehe auch ScaCap/action-surefire-report#31 -> scheint mit dem aktuellen Workflow nicht zu funktionieren, der offizielle Weg geht über einen workflow_run Trigger (Doku: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/). Unter deephaven/deephaven-core#547 ist eine gute Kopiervorlage, das sieht ganz verlockend aus.

@sgrell sgrell merged commit 8a86cbb into europace:main Apr 27, 2022
@sgrell
Copy link
Contributor

sgrell commented Apr 27, 2022

@gesellix endlich passt alles, danke für die Unterstützung 😄 Wie ist sind die Versionen gedacht, ist ja anscheinend kein Breaking-Change, aber ein Menge Kleinkram ist inzwischen zusammengekommen, ist es dann 1.3.0 oder 1.2.1?

@gesellix
Copy link
Contributor Author

@sgrell meine Empfehlung wäre für 1.3.0, weil das dann konsistent mit den anderen transitiven Dependencies wäre. Allerdings ist aus Nutzerperspektive des Plugins praktisch nichts passiert, eine 1.2.1 wäre also denkbar.

@gesellix gesellix deleted the upd branch April 27, 2022 13:04
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.

None yet

2 participants