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

Fix public suffix process and import #7700

Merged
merged 4 commits into from
Apr 22, 2023
Merged

Conversation

yschimke
Copy link
Collaborator

First step for #7699

Next automating in CI.

@yschimke
Copy link
Collaborator Author

yschimke commented Apr 1, 2023

friendly ping @swankjesse

*/
class PublicSuffixListGenerator(
projectRoot: Path = ".".toPath(),
val fileSystem: FileSystem = FileSystem.SYSTEM,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reference to okio.FileSystem will win my heart!

}

private suspend fun updateLocalFile() = withContext(Dispatchers.IO) {
client.newCall(request).executeAsync().use { response ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fancy

check(fileSystem.metadata(resources).isDirectory)
check(fileSystem.metadata(testResources).isDirectory)

updateLocalFile()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having each of these functions do withContext(Dispatchers.IO), rather than pulling that dispatcher switch up here is surprising? As-is it’s correct and good though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's that thing about making all suspend functions main safe, and that withContext(Dispatchers.IO) should be a no-op from Dispatchers.IO or Dispatchers.Default.

}
}

suspend fun main() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Such a great language feature

@yschimke yschimke merged commit 58fd29f into square:master Apr 22, 2023
@yschimke yschimke deleted the public_suffix branch May 27, 2023 11:26
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