Skip to content

Modern I/O article (issue #26) #66

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

Merged
merged 6 commits into from
Apr 25, 2024
Merged

Modern I/O article (issue #26) #66

merged 6 commits into from
Apr 25, 2024

Conversation

cayhorstmann
Copy link
Contributor

No description provided.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Feb 1, 2024
@cayhorstmann cayhorstmann mentioned this pull request Feb 2, 2024
Copy link
Contributor

@danthe1st danthe1st left a comment

Choose a reason for hiding this comment

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

Here are a few suggestions.
These are just suggestions, I am just an outside user interested in this repo.

Copy link
Contributor

@danthe1st danthe1st left a comment

Choose a reason for hiding this comment

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

I found other minor things that may still be worth considering.

Copy link
Contributor

@danthe1st danthe1st left a comment

Choose a reason for hiding this comment

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

Thank you very much for your article and your changes to it. I think it turned out great.

Copy link
Contributor

@nipafx nipafx left a comment

Choose a reason for hiding this comment

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

Hi Cay. 👋🏾 Looks good but there are a few small fixes to make. I proposed many of them below.

@carimura
Copy link
Member

@cayhorstmann sorry for the delay, let's get this merged! What does everyone think about making this article the 5th in the I/O series [1]?

[1] https://dev.java/learn/java-io/

@danthe1st
Copy link
Contributor

danthe1st commented Apr 24, 2024

I think it might be weird to have that article after Putting it All Together.
Maybe Putting it All Together should be adapted to also contain something from this article and put this article before?

Also, I just realized that the file here is just called index.md and doesn't have any metadata.

@carimura
Copy link
Member

I've already fixed the metadata locally. Just need a place to put the tutorial. We are leaning away from the series now.

@carimura
Copy link
Member

carimura commented Apr 24, 2024

ok we're considering some possible content reorg strategies but for now I'm thinking keep it where it's at: "Mastering the API" section as its own article, not as part of the I/O series.

@carimura
Copy link
Member

ok metadata added, ready for merge here cc @cayhorstmann @ammbra @JosePaumard.

Copy link
Member

@carimura carimura left a comment

Choose a reason for hiding this comment

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

LGTM -- looking for one more review from Java team on organization

@danthe1st
Copy link
Contributor

ok we're considering some possible content reorg strategies but for now I'm thinking keep it where it's at: "Mastering the API" section as its own article, not as part of the I/O series.

Should there be a table of contents?

@carimura
Copy link
Member

TOC should automatically build itself when missing

@carimura carimura merged commit a5166d8 into main Apr 25, 2024
@carimura
Copy link
Member

boom thanks all!

@carimura carimura deleted the modern-io branch April 25, 2024 14:20
@carimura carimura restored the modern-io branch April 25, 2024 14:20
@nipafx
Copy link
Contributor

nipafx commented May 17, 2024

Thanks for finishing this, guys! 👍🏾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants