Skip to content

Conversation

@sekikn
Copy link
Contributor

@sekikn sekikn commented Feb 14, 2022

Make sure you have checked all steps below.

Jira

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

No additional test since it's just a documentation fix.

Commits

  • My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain Javadoc that explain what it does

* Type-specific encoding
* Hive integration (deprecated)
* Pig integration
* Cascading integration
Copy link
Contributor

@shangxinli shangxinli Feb 14, 2022

Choose a reason for hiding this comment

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

Since the code is still there, do you think we can just add '(deprecated)' here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment, @shangxinli! I updated the PR.
Instead of removing the lines, I just added '(deprecated)' to parquet-cascading* and parquet-scrooge in README.md, just like parquet-hive.

@shangxinli
Copy link
Contributor

@sekikn Thanks for working on it! Just leave some minor comments.

@shangxinli
Copy link
Contributor

Thanks for addressing the comments! There is one left over. After that, I think we can merge it.

@sekikn
Copy link
Contributor Author

sekikn commented Feb 28, 2022

@shangxinli I'm afraid I can see just one comment at L69 of README.md...
Would you point to the comment left with a permalink or something? Maybe I don't understand GitHub's functionality or missed something.

@shangxinli
Copy link
Contributor

The only comment left is in the .gitignore file. Question is should we remove 'parquet-scrooge/.cache'? If the code for parquet-scrooge is still there, there may be needed to keep it.

@sekikn
Copy link
Contributor Author

sekikn commented Feb 28, 2022

Thank you for the elaboration, @shangxinli!
Let me confirm, I think the parquet-cascading* and parquet-scrooge directories were removed on the master branch in #888.
In addition, they had been renamed to "*-deprecated" before the removal, so the line in .gitignore in question has no effect.
So I'm not sure what "the code is still there" you mentioned means... Is my understanding correct?

@shangxinli
Copy link
Contributor

Thank you for the elaboration, @shangxinli! Let me confirm, I think the parquet-cascading* and parquet-scrooge directories were removed on the master branch in #888. In addition, they had been renamed to "*-deprecated" before the removal, so the line in .gitignore in question has no effect. So I'm not sure what "the code is still there" you mentioned means... Is my understanding correct?

Thanks @sekikn for checking!

@shangxinli shangxinli merged commit 4d062dc into apache:master Mar 4, 2022
@sekikn sekikn deleted the PARQUET-2121 branch March 5, 2022 02:17
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.

2 participants