Skip to content

Changes i told u about in skype#13

Merged
GodDragoner merged 5 commits into
GodDragoner:masterfrom
skier233:master
Nov 26, 2018
Merged

Changes i told u about in skype#13
GodDragoner merged 5 commits into
GodDragoner:masterfrom
skier233:master

Conversation

@skier233
Copy link
Copy Markdown
Collaborator

ddd

Copy link
Copy Markdown
Owner

@GodDragoner GodDragoner left a comment

Choose a reason for hiding this comment

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

Everything else looks fine 👍


inputBuffer.append(strLine);
inputBuffer.append('\n');
if (!strLine.equals(image.getName())) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why would you check that first? In the case of !added the file was not tagged previously which means we need to add it no matter whether the tags are empty right?
Because otherwise it will be an untagged and completely unknown image right?
You could also just check for "if(!added && !tagsToAdd.isEmpty())".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is because, if the strLine is just the name of the image, then it doesn't have a dress state or any tags and we don't want a picture in the tags file if it doesn't have any tags or a dress state.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why wouldn't we want an image without any tags inside the tags file? Does it matter?


lines.set(lines.indexOf(replaceLine), newLine);
TeaseLogger.getLogger().log(Level.INFO, "Added = " + added);
if (added) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You can change this into an else statement with the already existing !added statement

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yea. Thats fine and remove that logger message. That was just for testing.

@skier233
Copy link
Copy Markdown
Collaborator Author

should be ready for review and merge

@skier233
Copy link
Copy Markdown
Collaborator Author

Should be ready to merge this pull and push changes to release

Changes to your commit to improve performance and removed unnecessary code
@GodDragoner GodDragoner merged commit 7cd01b2 into GodDragoner:master Nov 26, 2018
GodDragoner added a commit that referenced this pull request Mar 1, 2020
Changes i told u about in skype
GodDragoner pushed a commit that referenced this pull request Mar 1, 2020
Merge pull request #13 from skier233/master
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