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

introduce downloadable sprites #33

Merged
merged 29 commits into from
Sep 15, 2024
Merged

Conversation

abhinayagarwal
Copy link
Contributor

Fixes #32

@johanvos johanvos requested a review from tiainen August 20, 2024 08:34
samples/introduction/pom.xml Outdated Show resolved Hide resolved
samples/nodes/pom.xml Show resolved Hide resolved
README.md Show resolved Hide resolved
@@ -45,12 +49,46 @@

public class Demo extends Application {

private StackPane root;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not needed, but if you want, add a logging.properties file, so tests can be done with fine level.

README.md Show resolved Hide resolved
Path tmpFilePath = getTmpFilePath(size);
if (Files.exists(tmpFilePath)) {
LOG.fine("Temporary download file found");
LOG.fine("Delete file and re-try downloading");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Increase log-level here. This is something that should only happen in rare occasions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want all logging to happen at info level or just this one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just that one.
While we are developing ourselves, we can easily increase the log level to check detailed logging.
But in case this is happening with users, and when they submit logs, only logs with INFO or higher are considered.

}

private Path getLockFilePath(int size) {
return Paths.get(String.format(LOCAL_PATH, commit)).resolve(size + ".downloading");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

does the extension .downloading makes sense or should I update it to .lck ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't really matter, but probably .lck or .tmp is better

Copy link
Collaborator

@jperedadnr jperedadnr left a comment

Choose a reason for hiding this comment

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

Looks good

@abhinayagarwal abhinayagarwal merged commit 16dd548 into gluonhq:main Sep 15, 2024
1 check passed
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.

Add an option to download emoji sprites on demand without packaging them
4 participants