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

Various fixes (mostly for the split) #107

Merged
merged 7 commits into from
Jun 24, 2020
Merged

Various fixes (mostly for the split) #107

merged 7 commits into from
Jun 24, 2020

Conversation

timholy
Copy link
Owner

@timholy timholy commented Jun 24, 2020

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 JuliaLang/Pkg.jl#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).

@timholy timholy mentioned this pull request Jun 24, 2020
16 tasks
@aminya
Copy link
Contributor

aminya commented Jun 24, 2020

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
Copy link
Owner Author

timholy commented Jun 24, 2020

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
Copy link
Owner Author

timholy commented Jun 24, 2020

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

@aminya
Copy link
Contributor

aminya commented Jun 24, 2020

@aminya
Copy link
Contributor

aminya commented Jun 24, 2020

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 JuliaLang/Pkg.jl#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
Copy link
Owner Author

timholy commented Jun 24, 2020

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.

src/SnoopCompile.jl Outdated Show resolved Hide resolved
@timholy
Copy link
Owner Author

timholy commented Jun 24, 2020

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
Copy link
Contributor

aminya commented Jun 24, 2020

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.
#108

timholy and others added 7 commits June 24, 2020 15:20
Co-authored-by: Amin Yahyaabadi <aminyahyaabadi74@gmail.com>
Also adds a clarification about sorting order in invalidations
Also switches to tilde bounds on package versions to be more restrictive.
Co-authored-by: Kristoffer Carlsson <kcarlsson89@gmail.com>
Co-authored-by: Amin Yahyaabadi <aminyahyaabadi74@gmail.com>
@timholy timholy merged commit 0aa0dae into master Jun 24, 2020
@timholy timholy deleted the teh/split_fixes branch June 24, 2020 21:11
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