Skip to content

Add support in Commons CSV for tracking byte positions during parsing#6

Merged
DarrenJAN merged 5 commits into
marklogic:1.12.1-marklogicfrom
DarrenJAN:apply-fix-on-release
Nov 5, 2024
Merged

Add support in Commons CSV for tracking byte positions during parsing#6
DarrenJAN merged 5 commits into
marklogic:1.12.1-marklogicfrom
DarrenJAN:apply-fix-on-release

Conversation

@DarrenJAN

@DarrenJAN DarrenJAN commented Oct 29, 2024

Copy link
Copy Markdown

Please note that it is required to use Java 11 to build and run tests.

Here are summaries of the modifications:

  1. Add our test data files to rat exclusions to avoid unapproved license checks in pom.xml
  2. Add CSVParser constructor
  3. Add CSVRecord constructor
    1. Remove mapping Since version 1.8, the mapping between the column header and the column index was removed from the serialized state (Note: Commons CSV: Latest version is 1.12.0)
  4. Add other Parser helper functions
    1. ExtendedBufferedReader()
    2. getBytesRead()
    3. getCharacterByte() in CSVRecord class
  5. Add new Junit test — src/test/java/org/apache/commons/csv/issues/JiraCsv196Test.java
    1. Update deprecated import package name (ie: import org.junit.Test; - > import org.junit.jupiter.api.Test; ; import org.junit.Assert → import static org.junit.jupiter.api.Assertions.assertEquals; )
    2. new InputStreamReader object to get test input
  6. Override the peek() functions in ExtendedBufferedReader class to avoid consuming the byte

Test result:
mvn
image
image
Pass unit tests and other restrictions

@yunzvanessa yunzvanessa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi Yuzhan,

The functionality looks good. Two things to focus on:

  1. Please go over all the comments you added since we would like to formalize it and attempt for a PR against the central repo.
  2. Please confirm the copyright info with Shawna and update them in the source files you are changing accordingly.

Comment thread src/main/java/org/apache/commons/csv/CSVFormat.java
Comment thread src/main/java/org/apache/commons/csv/CSVParser.java
@yunzvanessa

yunzvanessa commented Oct 31, 2024

Copy link
Copy Markdown

Can you also investigate the failing checks below and make sure they are okay?

@DarrenJAN DarrenJAN changed the title Apply Matt's fix to allow track byte position in 1.5.2-marklogic Add support in Commons CSV for tracking byte positions during parsing Nov 4, 2024
@DarrenJAN

Copy link
Copy Markdown
Author

Hi Yuzhan,

The functionality looks good. Two things to focus on:

  1. Please go over all the comments you added since we would like to formalize it and attempt for a PR against the central repo.
  2. Please confirm the copyright info with Shawna and update them in the source files you are changing accordingly.
  1. I updated some comments
  2. Should I confirm the copyright right now or wait until creating PR for the upstream project?

Comment thread src/main/java/org/apache/commons/csv/CSVParser.java Outdated

@yunzvanessa yunzvanessa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi Yuzhan,

Just a minor detail for you to change. Thanks

@DarrenJAN DarrenJAN merged commit 526ecc2 into marklogic:1.12.1-marklogic Nov 5, 2024
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