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

Issue 290: convert travis-ci to github-actions #294

Merged
merged 12 commits into from
Jan 19, 2021

Conversation

kaladay
Copy link
Contributor

@kaladay kaladay commented Jan 12, 2021

resolves #290
resolves #293

@kaladay kaladay marked this pull request as ready for review January 12, 2021 22:27
@coveralls
Copy link

coveralls commented Jan 13, 2021

Coverage Status

Coverage remained the same at 47.503% when pulling e953cde on sprint-action-290-yaml into 7b0aead on sprint-action-staging.

@kaladay kaladay linked an issue Jan 19, 2021 that may be closed by this pull request
4 tasks
@kaladay kaladay requested review from a user and rmathew1011 January 19, 2021 13:59
.github/workflows/build.yml Outdated Show resolved Hide resolved
uses: actions/cache@v2
with:
path: node_modules
key: ${{ runner.os }}-cache-node_modules-${{ hashFiles('**/package.json', '**/package-lock.json') }}
Copy link

Choose a reason for hiding this comment

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

As we are caching dependencies with node modules and m2 it makes sense to have at least one generic restore key. If we upgrade one dependency there is no reason to not bring in the cache for the remaining dependencies and let npm and mvn fetch the upgrade dependency.

From @wwelling:
  "As we are caching dependencies with node modules and m2 it makes sense to have at least one generic restore key. If we upgrade one dependency there is no reason to not bring in the cache for the remaining dependencies and let npm and mvn fetch the upgrade dependency."
uses: actions/cache@v2
with:
path: ~/.m2/repository
key: ${{ runner.os }}-cache-maven-${{ hashFiles('**/pom.xml') }}
Copy link

@ghost ghost Jan 19, 2021

Choose a reason for hiding this comment

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

This is not what I meant. I don't think this is correct either. We still want to lookup exact cache if package.json has not changed. But if package.json or pom.xml change it should fall back to a restore key such as:

         key: ${{ runner.os }}-cache-maven-${{ hashFiles('**/pom.xml') }}
         restore-keys: ${{ runner.os }}-cache-maven-

and

          key: ${{ runner.os }}-cache-node_modules-${{ hashFiles('**/package.json') }}
          restore-keys: ${{ runner.os }}-cache-node_modules-

This reverts commit 4820e5f.

This was a misinterpretation on my part.
@kaladay kaladay merged commit 87a7346 into sprint-action-staging Jan 19, 2021
@ghost ghost deleted the sprint-action-290-yaml branch February 3, 2021 14:10
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.

Should utilize githubactions cache for re-using both Maven and Node caches. Github Actions for SAGE:
3 participants