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

Added spark updater #2191

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

ashwinikalokhe
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jan 9, 2020

Codecov Report

Merging #2191 into master will decrease coverage by 1.3%.
The diff coverage is 74.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2191      +/-   ##
==========================================
- Coverage   71.75%   70.45%   -1.31%     
==========================================
  Files         136      148      +12     
  Lines       19978    23919    +3941     
==========================================
+ Hits        14336    16851    +2515     
- Misses       5642     7068    +1426
Impacted Files Coverage Δ
examples/pxScene2d/src/pxScene2d.cpp 68.53% <100%> (-1.77%) ⬇️
examples/pxScene2d/src/pxImage.h 78.57% <100%> (-1.43%) ⬇️
examples/pxScene2d/src/pxImageA.h 100% <100%> (ø) ⬆️
examples/pxScene2d/src/Spark.cpp 64.23% <100%> (+3.39%) ⬆️
examples/pxScene2d/src/pxImage.cpp 77.68% <25%> (-2.32%) ⬇️
examples/pxScene2d/src/pxImageA.cpp 84.16% <33.33%> (-1.8%) ⬇️
examples/pxScene2d/src/pxFont.h 94.11% <50%> (+0.36%) ⬆️
src/pxWindowUtil.cpp 95.4% <77.35%> (-4.6%) ⬇️
examples/pxScene2d/src/pxFont.cpp 73.03% <82.81%> (-4.59%) ⬇️
examples/pxScene2d/src/pxWebGL.h 83.6% <0%> (-14.48%) ⬇️
... and 100 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 360d5b7...438d1f2. Read the comment docs.

Copy link
Contributor

@mfiess mfiess left a comment

Choose a reason for hiding this comment

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

How will sparkUpdater.sh know that there is a Spark update available? It doesn't look like it checks the plist file in the cloud.

@ashwinikalokhe
Copy link
Contributor Author

How will sparkUpdater.sh know that there is a Spark update available? It doesn't look like it checks the plist file in the cloud.

@mfiess I have updated script to use UPDATE_URL to download software_update.plist and install spark from code_base url

Copy link
Contributor

@madanagopaltcomcast madanagopaltcomcast left a comment

Choose a reason for hiding this comment

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

Is it possible to have below similar blocks kept in common function:

CURRENT_PRODUCT_ID=$line
if [[ $CURRENT_PRODUCT_ID =~ $regex ]]
then
    CURRENT_PRODUCT_ID=${BASH_REMATCH[1]}
fi
idx=""

something like:
CURRENT_PRODUCT_ID=fn(line, pattern)
CURRENT_VERSION=fn(line, pattern)
CURRENT_CODEBASE =fn(line, pattern)

where pattern is like Codebase etc

Copy link
Contributor

@madanagopaltcomcast madanagopaltcomcast left a comment

Choose a reason for hiding this comment

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

1, We need to copy sparkUpdater.sh from macstuff to Spark.app during build. Change is needed in mkapp.sh
2, Command line arguments number need to be changed to odd numbers $3,$5,$7 and may need to check failure case like empty inputs etc.

@ashwinikalokhe
Copy link
Contributor Author

Thanks @madanagopalt for pointing that out as i missed changes in mkapp.sh

Copy link
Contributor

@madanagopalt madanagopalt left a comment

Choose a reason for hiding this comment

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

please add checks for empty inputs to .sh file.

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.

5 participants