Skip to content

Integrate gradle validation in normal test workflow#6001

Merged
koppor merged 1 commit into
masterfrom
validateWrapper
Feb 24, 2020
Merged

Integrate gradle validation in normal test workflow#6001
koppor merged 1 commit into
masterfrom
validateWrapper

Conversation

@tobiasdiez

Copy link
Copy Markdown
Member

I don't think we need an extra workflow for the gradle wrapper validation.

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for bigger UI changes)
  • Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 23, 2020
DBMS: "postgresql"
# This is https://github.com/marketplace/actions/gradle-wrapper-validation
# It ensures that the jar file is from gradle and not by a strange third party.
gradlevalidation:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should run before any gradle task otherwise it's nonsene

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They run in parallel as you see below...

@stefan-kolb stefan-kolb Feb 24, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe it's even easier to just include the step before running any Gradle actions, i.e. first step after checkout uses: gradle/wrapper-validation-action@v1. Dunno how long the task takes, but in that case we are sure that the test action fails which will be valued a lot more than just a parallel task that can be ignored?

But it's ok like that as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's unlikely that we get get a malicious file...

@koppor koppor merged commit 9cf9e7a into master Feb 24, 2020
@koppor koppor deleted the validateWrapper branch February 24, 2020 21:39
Siedlerchr added a commit that referenced this pull request Feb 26, 2020
* upstream/master: (21 commits)
  Squashed 'src/main/resources/csl-locales/' changes from 4fa753374e..d73af4a3fe
  Squashed 'src/main/resources/csl-styles/' changes from 9e81857..f03dece
  Add deep link to uni Basel
  Refine information on codacy
  New Crowdin translations (#6020)
  Fix l10n
  New Crowdin translations (#6016)
  Fix teamscale link - and sort list alphabetically
  Update code-quality.md
  Update link to codacy/JabRef
  Integrate gradle validation in normal test workflow (#6001)
  Update CleanupUrlFormatter.java
  Update URLUtil.java
  Fix modernizer warnings
  Remove unused error prone
  Use Java version class
  Remove commented out code
  Upgrade deprecated gradle API
  Add plugin scope
  New menu "Lookup"
  ...
Siedlerchr added a commit that referenced this pull request Mar 6, 2020
* upstream/master:
  Fix teamscale link - and sort list alphabetically
  Update code-quality.md
  Update link to codacy/JabRef
  Integrate gradle validation in normal test workflow (#6001)
  Fixed group drag-n-drop not working after inserting an article into a group (#6008)
  New Crowdin translations (#5998)
  Bump richtextfx from 0.10.3 to 0.10.4 (#6007)
  Bump unirest-java from 3.5.00 to 3.6.00 (#6006)
  Bump com.github.ben-manes.versions from 0.27.0 to 0.28.0 (#6004)
  Bump classgraph from 4.8.64 to 4.8.65 (#6003)
  Bump mockito-core from 3.2.4 to 3.3.0 (#6005)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants