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

Java mapper #300

Merged
merged 6 commits into from
Jul 9, 2019
Merged

Java mapper #300

merged 6 commits into from
Jul 9, 2019

Conversation

potiuk
Copy link
Collaborator

@potiuk potiuk commented Jul 9, 2019

Make sure you have checked all steps below and that above description is correct.

Github issues

  • My PR addresses the following Issue
    issue and references it in commit message.

Tests

  • My PR has unit tests testing the functionality (or I explained why it needs no tests)

Commits

  • My commits reference the issue in the message using: Fixes #XXX /Closes #XXX or similar.

Documentation

  • My PR adds documentation that describes how to use it (or I explained why it needs no docs)

@potiuk potiuk self-assigned this Jul 9, 2019
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@codecov
Copy link

codecov bot commented Jul 9, 2019

Codecov Report

Merging #300 into master will increase coverage by 0.04%.
The diff coverage is 98.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
+ Coverage   97.24%   97.29%   +0.04%     
==========================================
  Files         103      106       +3     
  Lines        4391     4546     +155     
==========================================
+ Hits         4270     4423     +153     
- Misses        121      123       +2
Impacted Files Coverage Δ
tests/converter/test_workflow_xml_parser.py 100% <ø> (ø) ⬆️
o2a/converter/workflow_xml_parser.py 98.43% <ø> (ø) ⬆️
o2a/converter/oozie_converter.py 100% <100%> (ø) ⬆️
tests/mappers/test_java_mapper.py 100% <100%> (ø)
o2a/converter/mappers.py 100% <100%> (ø) ⬆️
o2a/converter/workflow.py 90.24% <100%> (+1.35%) ⬆️
o2a/converter/constants.py 100% <100%> (ø) ⬆️
o2a/utils/file_utils.py 88.88% <88.88%> (ø)
o2a/mappers/java_mapper.py 98.38% <98.38%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e52e2f7...dc3232d. Read the comment docs.

o2a/converter/workflow.py Outdated Show resolved Hide resolved
o2a/converter/workflow.py Outdated Show resolved Hide resolved
o2a/mappers/java_mapper.py Outdated Show resolved Hide resolved
o2a/mappers/java_mapper.py Outdated Show resolved Hide resolved
o2a/mappers/java_mapper.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@turbaszek turbaszek left a comment

Choose a reason for hiding this comment

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

Just a comment about tags constants, otherwise good job 👌🏼

o2a/mappers/java_mapper.py Outdated Show resolved Hide resolved
@potiuk potiuk added the cla label Jul 9, 2019
@potiuk potiuk force-pushed the java-mapper branch 2 times, most recently from d1c78b4 to ff1c59f Compare July 9, 2019 12:11
@potiuk potiuk removed the cla label Jul 9, 2019
Copy link
Contributor

@sprzedwojski sprzedwojski left a comment

Choose a reason for hiding this comment

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

Overall looks good, but I do have some comments.

o2a/mappers/java_mapper.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
o2a/mappers/java_mapper.py Outdated Show resolved Hide resolved
o2a/templates/mapreduce.tpl Outdated Show resolved Hide resolved
o2a/utils/file_utils.py Outdated Show resolved Hide resolved
o2a/utils/file_utils.py Outdated Show resolved Hide resolved
o2a/utils/file_utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@sprzedwojski sprzedwojski left a comment

Choose a reason for hiding this comment

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

All looks good now, thanks! 💪

@potiuk potiuk merged commit 670c8eb into master Jul 9, 2019
@potiuk potiuk deleted the java-mapper branch July 22, 2019 11:56
ahidalgob pushed a commit that referenced this pull request Sep 22, 2023
Add Java mapper

GitOrigin-RevId: 60d10e8b385f8ec73a6c5734e9f4d6cd49a85016
Change-Id: Ibca5cfe68cb29e456014ddc3a8be779f70788c06
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.

5 participants