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

Adding multi-os and multi-version signatures support #71

Merged
merged 162 commits into from
Jun 15, 2020

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Feb 11, 2020

  • Redesigned the bot
  • using functions instead of macros for more stability
    • get the path to the example_script
    • automatic search for the script
      • combined the snooping methods together
  • more stability for deactivation/activation
  • Many new configurations and features such as:
  • Multiple OS signature generation support
  • Multiple Julia version support
  • YAML file: creating one consolidated PR for all of the OS/Versions
  • being resilient to the current directory
  • accepting precompile root path
  • accepting package path
  • add optional exhaustive checker (try catch eval all)
  • blacklist Main by default
  • add SnoopCompile_ENV for skipping a test in SnoopCompile environment
  • many error checks and warnings/info messages
  • tests (bot tests pass locally)
  • using GitHub actions for CI (except for docs)

Fixes #65
Fixes #55
Fixes #62
Fixes #76
Fixes #84
Fixes #80
Fixes #78

Review Tips

  • I think running the tests is the easiest way for reviewing (test/bot.jl)
  • This PR got huge over time. I can give a walkthrough in a Skype call if it is needed.

@aminya
Copy link
Contributor Author

aminya commented Feb 11, 2020

@timholy I decided to have a @static if should_precompile that we put all the precompile scripts inside.
This allows us just to set should_precompile to true or false which is much easier than commenting and uncommenting the lines of code.

https://github.com/aminya/SnoopCompile.jl/blob/d9513a7052b6a3c24cdd04d0c792850d9bb89969/src/bot/precompileInclude.jl#L192

However, to keep things clean, I am wondering if we can write the code in a separate file and then include it. But does @static work the same way inside that file as it was outside in the main file of a package?

The file can be something like src/precompile_includer.jl.

@aminya aminya force-pushed the multios branch 2 times, most recently from 4e27970 to f02a441 Compare February 16, 2020 11:23
@aminya
Copy link
Contributor Author

aminya commented Jun 14, 2020

@timholy If you agree let's merge this into the master branch. This PR is so big, and managing it is getting out of the hands.

The only two remaining things are:

By the way, don't forget to remove the Appveyor app from the settings

@timholy timholy merged commit 4cf5fe6 into timholy:master Jun 15, 2020
@timholy
Copy link
Owner

timholy commented Jun 15, 2020

Tada! 🎆 Thank you for your patience and persistence!

@aminya
Copy link
Contributor Author

aminya commented Jun 15, 2020

Yay!

You're welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants