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

Do not git ignore job scheduler dir, used. #348

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

dblock
Copy link
Member

@dblock dblock commented Dec 31, 2021

Signed-off-by: dblock [email protected]

Description

When updating to 1.2.4 a .zip file was not git add-ed, had to do it manually.

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock dblock requested a review from a team December 31, 2021 16:13
ylwu-amzn
ylwu-amzn previously approved these changes Jan 7, 2022
Copy link
Collaborator

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM. The CI workflow failure fixed in PR #350
You can rebase your branch on main to pass CI workflow. But as this PR just change .gitignore file, I think it's safe to skip the CI workflow. So it's up to you.

@amitgalitz
Copy link
Member

Are there any downsides to having it not be a part of .gitignore and potentially committing it when developing with an older job-scheduler build. I suppose that wont happen often but just wondering if you had any other thoughts on this as it was explicitly added to .gitignore in the past

@codecov-commenter
Copy link

Codecov Report

Merging #348 (af35981) into main (06509d5) will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #348      +/-   ##
============================================
+ Coverage     77.57%   77.71%   +0.13%     
- Complexity     4008     4019      +11     
============================================
  Files           295      295              
  Lines         17188    17188              
  Branches       1816     1816              
============================================
+ Hits          13334    13357      +23     
+ Misses         3005     2986      -19     
+ Partials        849      845       -4     
Flag Coverage Δ
plugin 77.71% <ø> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...java/org/opensearch/ad/task/ADBatchTaskRunner.java 78.11% <0.00%> (-3.65%) ⬇️
...ain/java/org/opensearch/ad/task/ADTaskManager.java 76.45% <0.00%> (-0.23%) ⬇️
...opensearch/ad/indices/AnomalyDetectionIndices.java 68.35% <0.00%> (-0.19%) ⬇️
...rch/ad/transport/AnomalyResultTransportAction.java 80.18% <0.00%> (+0.68%) ⬆️
...rch/ad/transport/ForwardADTaskTransportAction.java 97.45% <0.00%> (+3.38%) ⬆️
...va/org/opensearch/ad/feature/SearchFeatureDao.java 86.64% <0.00%> (+3.85%) ⬆️
...c/main/java/org/opensearch/ad/util/ParseUtils.java 71.59% <0.00%> (+4.28%) ⬆️
...g/opensearch/ad/model/DetectorValidationIssue.java 54.38% <0.00%> (+7.01%) ⬆️
...opensearch/ad/transport/AnomalyResultResponse.java 84.09% <0.00%> (+12.12%) ⬆️

@dblock dblock requested a review from a team January 7, 2022 19:35
@dblock
Copy link
Member Author

dblock commented Jan 7, 2022

Are there any downsides to having it not be a part of .gitignore and potentially committing it when developing with an older job-scheduler build. I suppose that wont happen often but just wondering if you had any other thoughts on this as it was explicitly added to .gitignore in the past

AFAIK bwc CI won't work without these

The fact that any dependency JARs are checked into the project are a problem. I opened #352.

@dblock dblock merged commit 1714976 into opensearch-project:main Jan 7, 2022
@dblock dblock deleted the do-not-ignore branch January 7, 2022 19:43
ylwu-amzn pushed a commit to ylwu-amzn/anomaly-detection-2 that referenced this pull request Jan 11, 2022
ylwu-amzn pushed a commit that referenced this pull request Jan 12, 2022
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.

4 participants