timholy / SnoopCompile.jl

Provide insights about latency (TTFX) for Julia packages
https://timholy.github.io/SnoopCompile.jl/dev/
Other
309 stars 48 forks source link

Various fixes (mostly for the split) #107

Closed timholy closed 4 years ago

timholy commented 4 years ago

This fixes a number of issues, most of which are leftovers from splitting the package and most of which only concern the docs. The reason change fixes a bug when invalidations stem from both method insertions and method deletions, and the Manifest.toml file is a workaround for https://github.com/JuliaLang/Pkg.jl/issues/1874. We'll have to remember to update the Manifest before every PR, presumably, and I'm not sure it will be robust across Julia versions, but I think it's the best we can do for now.

@aminya, if #105 is still an issue even after this, you may want to add some additional using SnoopCompileBot: some_private_function statements to SnoopCompile, see the existing models. I'll wait for your comments on this before merging (feel free to submit a PR against this branch).

aminya commented 4 years ago

I think we should restart the failed jobs. That was probably a connection issue.

We can add this fail-fast to prevent other jobs from stopping when one fails

    strategy:
      fail-fast: false
      matrix:
timholy commented 4 years ago

Looks like doing the dev from within the ci.yml script works well. Thanks for all the suggestions, everyone! I'll merge this soon if there are no objections. @aminya, be sure to test Bot functionality and add any more using entries you need.

timholy commented 4 years ago

Instead of restarting, any additions you want? Might as well test those as the same time.

aminya commented 4 years ago

I am testing this branch here: https://github.com/JuliaMusic/MusicXML.jl/actions?query=workflow%3ASnoopCompile

aminya commented 4 years ago

This fails. https://github.com/JuliaMusic/MusicXML.jl/runs/804800745?check_suite_focus=true#step:4:91

   Updating registry at `~/.julia/registries/General`
   Updating git-repo `https://github.com/JuliaRegistries/General.git`
  Resolving package versions...
ERROR: Unsatisfiable requirements detected for package SnoopCompileBot [1d5e0e55]:
 SnoopCompileBot [1d5e0e55] log:
 ├─possible versions are: 1.6.0 or uninstalled
 └─restricted to versions 1.6.1-1.6 by SnoopCompile [aa65fe97] — no versions left
   └─SnoopCompile [aa65fe97] log:
     ├─possible versions are: 1.6.1 or uninstalled
     └─SnoopCompile [aa65fe97] is fixed to version 1.6.1

This is about my point about https://github.com/JuliaLang/Pkg.jl/issues/1874, which is why we should change all of the codes everywhere, instead of fixing it in one place (deps/build.jl).

Now I need to change SnoopCompile.yml to test this branch!

timholy commented 4 years ago

Packages shouldn't dev SnoopCompile. I'm asking you to run it locally on your machine and see if all the names are in their proper places now with no changes to the old snoopbot scripts. Once you tell me that, we can merge & release and then MusicXML.jl will have no problem getting a consistent 1.6.1 release.

In other words, run MusicXML's scripts locally and see if they work. Don't change its script in the package.

timholy commented 4 years ago

If that's it...I have a commit queued to add the dev strategy to Documenter.yml, but let's see how this does otherwise through CI. If you have nothing else, then I think this may be good to go.

aminya commented 4 years ago

I tested this on MusicXML locally and it works now.

I made a dev script, which allows me to dev SnoopCompile for different environments quickly. https://github.com/timholy/SnoopCompile.jl/pull/108