Skip to content

Add build info to README.#304

Merged
rdblue merged 1 commit intoapache:masterfrom
timmylicheng:fix-issue-286
Jul 29, 2019
Merged

Add build info to README.#304
rdblue merged 1 commit intoapache:masterfrom
timmylicheng:fix-issue-286

Conversation

@timmylicheng
Copy link
Copy Markdown
Contributor

Comment thread README.md Outdated
Iceberg is built using Gradle 5.2.1.

* To invoke a build and run tests, run: ./gradlew build
* To skip tests, run: ./gradlew build -x test
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is cool.

Copy link
Copy Markdown
Contributor

@fbocse fbocse Jul 22, 2019

Choose a reason for hiding this comment

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

Next to skipping tests we could also document how to skip checkstyle violations (it usually helps lower the barrier for trying code changes) ./gradlew clean build -x test -x check. Wdyt?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@fbocse Sounds good. Adding it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checkstyle violations will be caught in CI testing and block PRs, so I'm not sure that excluding that check is a good idea. Maybe just note that this is okay for development, but will cause PRs to fail validation.

@chenjunjiedada
Copy link
Copy Markdown
Collaborator

@timmylicheng you may need to close and reopen PR to trigger the CI again.

Comment thread README.md Outdated

Iceberg is built using Gradle 5.2.1.

* To invoke a build and run tests, run: ./gradlew build
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will look nicer if you use code formatting (backticks):

* To invoke a build and run tests, run: `./gradlew build`

I'd also simplify it by removing the ", run" part on all of these:

* To invoke a build and run tests: `./gradlew build`

It will look like this when rendered:

  • To invoke a build and run tests: ./gradlew build

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Kk. I will update it.

Comment thread README.md Outdated
Iceberg is built using Gradle 5.2.1.

* To invoke a build and run tests: `./gradlew build`
* To locally check sanity: `./gradlew check`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe "run checkstyle"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? I see CI is using ./gradlew check.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"check sanity" isn't a very clear description of why someone would use this task. Maybe it would be better to keep it simple and have just ./gradlew build and ./gradlew build -x test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Aite

Comment thread README.md Outdated
* To invoke a build and run tests: `./gradlew build`
* To locally check sanity: `./gradlew check`
* To skip tests: `./gradlew build -x test`
* To skip tests and checkstyle (make sure you mean it): `./gradlew clean build -x test -x check`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's remove this. I think it is clear how to skip steps from the -x test example. The note that checkstyle is run in CI and PRs will fail is needed if this is here, but seems needlessly long to me.

Copy link
Copy Markdown
Contributor Author

@timmylicheng timmylicheng Jul 26, 2019

Choose a reason for hiding this comment

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

Ok.
I personally tried build without checkstyle initially. So when @fbocse brought it up, I thought maybe new users would wanna know.

@timmylicheng timmylicheng reopened this Jul 29, 2019
@rdblue rdblue merged commit 5366efe into apache:master Jul 29, 2019
@rdblue
Copy link
Copy Markdown
Contributor

rdblue commented Jul 29, 2019

Merged. Thanks for contributing this, @timmylicheng!

danielcweeks pushed a commit that referenced this pull request Aug 1, 2019
* Add argument validation to HadoopTables#create (#298)

* Install source JAR when running install target (#310)

* Add projectStrict for Dates and Timestamps (#283)

* Correctly publish artifacts on JitPack (#321)

The Gradle install target produces invalid POM files that are missing
the dependencyManagement section and versions for some dependencies.
Instead, we directly tell JitPack to run the correct Gradle target.

* Add build info to README.md (#304)

* Convert Iceberg time type to Hive string type (#325)

* Add overwrite option to write builders (#318)

* Fix out of order Pig partition fields (#326)

* Add mapping to Iceberg for external name-based schemas (#338)

* Site: Fix broken link to Iceberg API (#333)

* Add forTable method for Avro WriteBuilder (#322)

* Remove multiple literal strings check rule for scala (#335)

* Fix invalid javadoc url in README.md (#336)

* Use UnicodeUtil.truncateString for Truncate transform. (#340)

This truncates by unicode codepoint instead of Java chars.

* Refactor metrics tests for reuse (#331)

* Spark: Add support for write-audit-publish workflows (#342)

* Avoid write failures if metrics mode is invalid (#301)

* Fix truncateStringMax in UnicodeUtil (#334)

Fixes #328, fixes #329.

Index to codePointAt should be the offset calculated by code points

* [Vectorization] Added batch sizing, switched to BufferAllocator, other minor style fixes.
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