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

PR to fix https://github.com/salesforce/dockerfile-image-update/issues/236, to bring in support for sending PRs to docker-compose files #687

Merged
merged 7 commits into from
Jan 24, 2023

Conversation

pchugh17
Copy link
Contributor

@pchugh17 pchugh17 commented Dec 6, 2022

Following changes are being implemented in this PR:

  • Support to send PRs to docker-compose files along with Dockerfile. This choice is configurable from command line parameter.
  • In docker-compose, PRs will not be sent if tag is invalid. Also PR will not be sent if tag.equals("latest")
  • mockito-inline is replaced with Mockito-core as prior is deprecated.

@github-actions github-actions bot added the tests Improvements / maintenance to tests label Dec 6, 2022
search.filename("Dockerfile");
search.filename("docker-compose");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move these filenames to come from CLI where we have a default list of "Dockerfile", "docker-compose" there? Then this part of code would do search.filename on a list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

assertEquals(new ImageKeyValuePair(fromInstruction).hasADifferentTag(tagToCheck), expectedResult);
}

/* @DataProvider
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to keep this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@pchugh17 pchugh17 added WIP feature New features and improvements labels Jan 3, 2023
Additionally updated to ignore sending PR for latest tag in docker-compose.
Also ignore changing invalid tags which include variables like ${ITEST}/${version}
Updated mockito-all to mockito-core as prior is deprecated.
@codecov
Copy link

codecov bot commented Jan 6, 2023

Codecov Report

Merging #687 (696e7e8) into main (0744b1f) will increase coverage by 1.07%.
The diff coverage is 95.27%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #687      +/-   ##
============================================
+ Coverage     83.33%   84.40%   +1.07%     
- Complexity      320      374      +54     
============================================
  Files            26       27       +1     
  Lines          1140     1244     +104     
  Branches        137      164      +27     
============================================
+ Hits            950     1050     +100     
- Misses          155      157       +2     
- Partials         35       37       +2     
Impacted Files Coverage Δ
...esforce/dockerfileimageupdate/utils/Constants.java 100.00% <ø> (ø)
.../salesforce/dockerfileimageupdate/CommandLine.java 62.93% <83.33%> (+1.32%) ⬆️
...kerfileimageupdate/utils/DockerfileGitHubUtil.java 85.93% <88.23%> (+0.22%) ⬆️
...dockerfileimageupdate/model/ImageKeyValuePair.java 98.36% <98.36%> (ø)
...rce/dockerfileimageupdate/model/GitForkBranch.java 91.66% <100.00%> (+2.77%) ⬆️
...fileimageupdate/process/ForkableRepoValidator.java 97.82% <100.00%> (+0.09%) ⬆️
...leimageupdate/process/GitHubPullRequestSender.java 89.13% <100.00%> (ø)
...eimageupdate/search/GitHubImageSearchTermList.java 98.18% <100.00%> (+0.03%) ⬆️
...ce/dockerfileimageupdate/subcommands/impl/All.java 89.70% <100.00%> (+0.15%) ⬆️
.../dockerfileimageupdate/subcommands/impl/Child.java 100.00% <100.00%> (ø)
... and 1 more

@pchugh17 pchugh17 requested a review from afalko January 6, 2023 11:15
afalko
afalko previously approved these changes Jan 9, 2023
@pchugh17 pchugh17 removed the WIP label Jan 23, 2023
@pchugh17 pchugh17 changed the title Initial Commit to support https://github.com/salesforce/dockerfile-image-update/issues/236 PR to fix https://github.com/salesforce/dockerfile-image-update/issues/236, to bring in support for sending PRs to docker-compose files Jan 23, 2023
@pchugh17 pchugh17 requested a review from afalko January 23, 2023 12:06
@DataProvider
public Object[][] imageNameSpecifiedBranchAndFilenameSearched() {
return new Object[][]{
/*{"docker.io/some/container", "", "blah", "dockerfile", "blah"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need these commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was added for some debugging, uncommented now. Thanks @afalko

if (!validTag.matcher(tag.trim()).matches()) {
log.warn("{} is not a valid value for image tag. So will be ignored!", tag);
return false;
} else if (tag.trim().equals("latest")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we'll want to parameterize this at some point. For now this is good to have in this PR

@afalko afalko merged commit 890e120 into salesforce:main Jan 24, 2023
@pchugh17 pchugh17 deleted the issue_236 branch January 24, 2023 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features and improvements tests Improvements / maintenance to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants