Scripts changes #747
Merged
Scripts changes #747
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also remove temporary resized, uploaded file. Other minor changes.
Added check for missing directory. Remove unnecessary code (every other script assumes $EXTENSION exists). Removed unneeded "cd $ALLSKY_HOME". Set defaults in one place so it's easier to remove them in the future when we know they are in the unified configuration file. Misc minor changes.
Didn't handle incorrect arguments very well, e.g.: `generateForDay.sh -t` would say "-t not found`.
linuxkidd
reviewed
Nov 1, 2021
scripts/copy_notification_image.sh
Outdated
| RET=$? | ||
|
|
||
| # If we created a temporary copy, delete it. | ||
| [ "${RESIZE_UPLOADS}" = "true" ] && rm -f "${IMAGE_TO_USE}" |
There was a problem hiding this comment.
I realize this contains ${ALLSKY_TMP}/${FULL_FILENAME}, but let's use the expanded version ( as I provided it here ), instead of ${IMAGE_TO_USE}. This helps eliminate confusion during future code reviews.
There was a problem hiding this comment.
@linuxkidd, not sure I understand your comment. Almost all the scripts use ${IMAGE_TO_USE} as the name of the current image being processed. This makes it easier to change locations of the file (like putting in in $ALLSKY_TMP in the near future) since only one line per script needs to change.
What do you mean by "as I provided it here"? I only see a few lines of newly added code in your reply that removes a temporary file.
IMGDIR is the old name; changed to use new name IMAGE_DIR.
Using IMAGE_TO_USE for the temporary file was confusing and dangerous. It now uses TEMP_FILE for the uploaded file being resized.
linuxkidd
approved these changes
Nov 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Misc fixes and changes.