Skip to content

Improve core archive info #288

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

Merged
merged 3 commits into from
Sep 7, 2018

Conversation

facchinm
Copy link
Member

@facchinm facchinm commented Sep 4, 2018

No description provided.

@ArduinoBot
Copy link
Contributor

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-288.zip

ℹ️ To test this build:

  1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

if err == nil {
logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_ARCHIVING_CORE_CACHE, targetArchivedCore)
} else {
logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_CORE_CACHE_UNAVAILABLE, ctx.ActualPlatform.PlatformId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This blames any error while copying the file on the core maintainers. I suppose the intended case is where the core does not use the proper .a filename, so the copy cannot find it? If so, then perhaps a separate check should be made to see if the .a filename exists and give this error if it does not? If the file does exist, but the copy fails, a new error message should probably be shown?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd also add arduino.ar as a possible target but this clashes badly with specifications at http://goo.gl/QdCUjo .
I think that adding this check will make life terrible for maintainers of "incompatible" cores for a bit but will enormously speedup the adoption of proper naming (esp8266 just merged it in mainline esp8266/Arduino@85e6809#diff-d638a47d1ca0e2a5585fb34c7d7fb0f9L95 )

Copy link
Collaborator

Choose a reason for hiding this comment

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

Terrible in what way? Because users will start pestering them? Or because their incompatible cores will break or become slower (I can't see why or how)?

Copy link
Member Author

Choose a reason for hiding this comment

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

The first one 🙂

@facchinm facchinm merged commit f3748a5 into arduino:master Sep 7, 2018
@ArduinoBot
Copy link
Contributor

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-288.zip

ℹ️ To test this build:

  1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

@cmaglie cmaglie deleted the improve_core_archive_info branch September 7, 2018 13:53
@cmaglie cmaglie added this to the 1.4.1 milestone Sep 10, 2018
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.

4 participants