Addressing review comments

v2.0.0 pull request bot pathof timholy#49 (comment) rename snoopiBenchBot ro snoopiBench timholy#49 (comment) Doc reference Fixes timholy#49 (comment) Naming convention =@. timholy#49 (comment) Benchmarking the infer time of the tests timholy#49 (comment) code quoting const UStrings timholy#49 (comment) timholy#49 (comment) file name typo timholy#49 (comment) Link updated timholy#49 (comment) typo timholy#49 (comment) __precompile__ timholy#49 (comment) Base.read and Base.write timholy#49 (comment) timholy#49 (comment) UString timholy#49 (comment) precompile include clarification timholy#49 (comment) Experimental timholy#49 (comment) Using isfile timholy#49 (comment) test dep Clarification timholy#49 (comment) and timholy#49 (comment) Comitter comment Fixes timholy#49 (comment) Base.write instead of (de)activated.jl in tests Fixes timholy#49 (comment)
aminya · Jan 6, 2020 · e878748 · e878748
1 parent 2ebc8d8
commit e878748
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 93 deletions.
diff --git a/docs/src/bot.md b/docs/src/bot.md
@@ -1,4 +1,4 @@
-# SnoopCompile Bot
+# SnoopCompile Bot (EXPERIMENTAL)
 
 You can use SnoopCompile bot to automatically and continuously create precompile files.
 
@@ -7,7 +7,7 @@ One should add 3 things to a package to make the bot work:
 ----------------------------------
 
 
-- workflow file:
+- Workflow file:
 
 create a workflow file with this path in your repository `.github/workflows/SnoopCompile.yml` and use the following content:
 
@@ -32,9 +32,9 @@ jobs:
         with:
           version: ${{ matrix.julia-version }}
       - name: Install dependencies
-        run: julia --project=@. -e 'using Pkg; Pkg.instantiate();'
+        run: julia --project -e 'using Pkg; Pkg.instantiate();'
       - name : Add SnoopCompile and current package
-        run: julia -e 'using Pkg; Pkg.add(PackageSpec(url = "https://github.com/aminya/SnoopCompile.jl", rev ="packageSnooper")); Pkg.develop(PackageSpec(; path=pwd()));'
+        run: julia -e 'using Pkg; Pkg.add("SnoopCompile"); Pkg.develop(PackageSpec(; path=pwd()));'
       - name: Install Test dependencies
         run: julia -e 'using SnoopCompile; SnoopCompile.addtestdep()'
       - name: Generating precompile files
@@ -44,36 +44,37 @@ jobs:
 
       # https://github.com/marketplace/actions/create-pull-request
       - name: Create Pull Request
-        uses: peter-evans/create-pull-request@v2-beta
+        uses: peter-evans/create-pull-request@v2.0.0
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           commit-message: Update precompile_*.jl file
-          committer: Amin Yahyaabadi <aminyahyaabadi74@gmail.com>
+          committer: YOUR NAME <yourEmail@something.com> # Change `committer` to your name and your email.
           title: '[AUTO] Update precompile_*.jl file'
           labels: SnoopCompile
           branch: create-pull-request/SnoopCompile
       - name: Check output environment variable
         run: echo "Pull Request Number - ${{ env.PULL_REQUEST_NUMBER }}"
 ```
-If your examples or tests have dependencies, you should add a `Test.toml` to your test folder.
-
-`Install Test dependencies` step is only needed if you have test dependencies other than Test. Otherwise, you should comment it.
+`Install Test dependencies` step is only needed if you have test dependencies other than Test. Otherwise, you should comment it. In this case, if your examples or tests have dependencies, you should add a `Test.toml` to your test folder.
 
 ```yaml
-# - name: Install Test dependencies
-#  run: julia -e 'using SnoopCompile; SnoopCompile.addtestdep()'
+- name: Install Test dependencies
+  run: julia -e 'using SnoopCompile; SnoopCompile.addtestdep()'
 ```
 
 For example for MatLang package:
 
 [Link](https://github.com/juliamatlab/MatLang/blob/master/.github/workflows/SnoopCompile.yml)
 
-Change `committer` to your name and your email.
-
 ----------------------------------
 
 
-- Add a `snoopCompile.jl` file under `deps/SnoopCompile`. The content of the file can be the examples that call the package functions:
+- Precompile script
+
+Add a `snoopCompile.jl` file under `deps/SnoopCompile`. The content of the file should be a script that "exercises" the functionality you'd like to precompile. One option is to use your package's `"runtests.jl"` file, or you can write a custom script for this purpose.
+
+
+For example, some examples that call the functions:
 
 ```julia
 using SnoopCompile
@@ -88,7 +89,7 @@ end
 ```
 [Ref]( https://github.com/juliamatlab/MatLang/blob/master/deps/SnoopCompile/snoopCompile.jl)
 
-or If you do not have additional examples, you can use your runtests.jl file:
+or if you do not have additional examples, you can use your runtests.jl file using this syntax:
 
 ```julia
 using SnoopCompile
@@ -97,9 +98,15 @@ using SnoopCompile
 @snoopiBot "MatLang"
 ```
 
+[Also look at this](https://timholy.github.io/SnoopCompile.jl/stable/snoopi/#Precompile-scripts-1)
+
 ----------------------------------
 
-- Two lines of code that includes the precompile file in your main module. It is better to have these lines commented, if you want to continuously develop and change your package offline. Only merging pull requests online will uncomment the lines automatically:
+- Include precompile signatures
+
+Two lines of (commented) code that includes the precompile file in your main module.
+
+It is better to have these lines commented to continuously develop and change your package offline. snoopiBot will find these lines of code and will uncomment them in the created pull request. If they are not commented the bot will leave it as is in the pull request:
 
 ```julia
 # include("../deps/SnoopCompile/precompile/precompile_MatLang.jl")
@@ -109,6 +116,9 @@ using SnoopCompile
 [Ref](https://github.com/juliamatlab/MatLang/blob/072ff8ed9877cbb34f8583ae2cf928a5df18aa0c/src/MatLang.jl#L26)
 
 
+----------------------------------
+
+
 ## Benchmark
 
 To measure the effect of adding precompile files. Add a `snoopBenchmark.jl`. The content of this file can be the following:
@@ -117,14 +127,14 @@ Benchmarking the load infer time
 ```julia
 println("loading infer benchmark")
 
-@snoopiBenchBot "MatLang" using MatLang
+@snoopiBench "MatLang" using MatLang
 ```
 
 Benchmarking the example infer time
 ```julia
 println("examples infer benchmark")
 
-@snoopiBenchBot "MatLang" begin
+@snoopiBench "MatLang" begin
     using MatLang
     examplePath = joinpath(dirname(dirname(pathof(MatLang))), "examples")
     # include(joinpath(examplePath,"Language_Fundamentals", "usage_Entering_Commands.jl"))
@@ -135,7 +145,7 @@ end
 
 Benchmarking the tests:
 ```julia
-@snoopiBenchBot "MatLang"
+@snoopiBench "MatLang"
 ```
 [Ref](https://github.com/juliamatlab/MatLang/blob/master/deps/SnoopCompile/snoopBenchmark.jl)
 

diff --git a/src/bot.jl b/src/bot.jl
@@ -1,18 +1,18 @@
-export precompileActivator, precompileDeactivator, precompilePather, @snoopiBot, @snoopiBenchBot, BotConfig
+export precompile_activator, precompile_deactivator, precompile_pather, @snoopiBot, @snoopiBench, BotConfig
 
-UStrings = Union{AbstractString,Regex,AbstractChar}
+const UStrings = Union{AbstractString,Regex,AbstractChar}
 ################################################################
 """
     BotConfig
 
-Config object that holds the options and configuration for the snoopCompile bot. This object is fed to the `@snoopiBot`.
+Config object that holds the options and configuration for the SnoopCompile bot. This object is fed to the `@snoopiBot`.
 
 # Arguments:
-- packageName::String
-- subst::Vector{Pair{UStrings, UStrings}} : to replace a packages precompile setences with another's package like ["ImageTest" => "Images"]
-- blacklist::Vector{UStrings} : to remove some precompile sentences
+- `packageName::String`
+- `subst::Vector{Pair{UStrings, UStrings}}` : to replace a packages precompile setences with another's package like `["ImageTest" => "Images"]`
+- `blacklist::Vector{UStrings}` : to remove some precompile sentences
 
-# Ustrings ==  Union{AbstractString,Regex,AbstractChar} # every string like type that replace() has a method for.
+`const UStrings ==  Union{AbstractString,Regex,AbstractChar}` # every string like type that `replace()` has a method for.
 """
 struct BotConfig
     packageName::String
@@ -25,6 +25,6 @@ function BotConfig(packageName::String; subst::Vector{Pair{T1, T2}} where {T1<:U
 end
 
 include("bot/botutils.jl")
-include("bot/precomileInclude.jl")
+include("bot/precompileInclude.jl")
 include("bot/snoopiBot.jl")
-include("bot/snoopiBenchBot.jl")
+include("bot/snoopiBench.jl")
diff --git a/src/bot/botutils.jl b/src/bot/botutils.jl
@@ -6,10 +6,9 @@ Should be removed once Pkg allows adding test dependencies to the current enviro
 Used in Github Action workflow yaml file
 """
 function addtestdep()
-    local testToml
-    try
+    if isfile("test/Test.toml")
         testToml = Pkg.Types.parse_toml("test/Test.toml")
-    catch
+    else
         error("please add a Test.toml to the /test directory for test dependencies")
     end
 

diff --git a/src/bot/precomileInclude.jl → src/bot/precompileInclude.jl b/src/bot/precomileInclude.jl → src/bot/precompileInclude.jl
@@ -1,26 +1,26 @@
 
 """
-    precompilePather(packageName::String)
+    precompile_pather(packageName::String)
 
 To get the path of precompile_packageName.jl file
 
 Written exclusively for SnoopCompile Github actions.
 # Examples
 ```julia
-precompilePath, precompileFolder = precompilePather("MatLang")
+precompilePath, precompileFolder = precompile_pather("MatLang")
 ```
 """
-function precompilePather(packageName::String)
+function precompile_pather(packageName::String)
     return "\"../deps/SnoopCompile/precompile/precompile_$packageName.jl\"",
     "$(pwd())/deps/SnoopCompile/precompile/"
 end
 
-precompilePather(packageName::Symbol) = precompilePather(string(packageName))
-precompilePather(packageName::Module) = precompilePather(string(packageName))
+precompile_pather(packageName::Symbol) = precompile_pather(string(packageName))
+precompile_pather(packageName::Module) = precompile_pather(string(packageName))
 
 ################################################################
 
-function precompileRegex(precompilePath)
+function precompile_regex(precompilePath)
     # https://stackoverflow.com/questions/3469080/match-whitespace-but-not-newlines
     # {1,} for any number of spaces
     c1 = Regex("#[^\\S\\r\\n]{0,}include\\($(precompilePath)\\)")
@@ -32,19 +32,19 @@ end
 ################################################################
 
 """
-    precompileActivator(packagePath, precompilePath)
+    precompile_activator(packagePath, precompilePath)
 
 Activates precompile of a package by adding or uncommenting include() of *.jl file generated by SnoopCompile and _precompile_().
 
+packagePath is the same as `pathof`. However, `pathof(module)` isn't used to prevent loadnig the package.
+
 Written exclusively for SnoopCompile Github actions.
 """
-function precompileActivator(packagePath::String, precompilePath::String)
+function precompile_activator(packagePath::String, precompilePath::String)
 
-    file = open(packagePath,"r")
-    packageText = Base.read(file, String)
-    close(file)
+    packageText = Base.read(packagePath, String)
 
-    c1, c2, a1, a2 = precompileRegex(precompilePath)
+    c1, c2, a1, a2 = precompile_regex(precompilePath)
 
     # Checking availability of _precompile_ code
     commented = occursin(c1, packageText) && occursin(c2, packageText)
@@ -58,9 +58,8 @@ function precompileActivator(packagePath::String, precompilePath::String)
                      ),
                      init = packageText)
 
-                     file = open(packagePath,"w")
-                     Base.write(file, packageEdited)
-                     close(file)
+                     Base.write(packagePath, packageEdited)
+
         println("precompile is activated")
     elseif available
         # do nothing
@@ -76,19 +75,19 @@ function precompileActivator(packagePath::String, precompilePath::String)
 end
 
 """
-    precompileDeactivator(packagePath, precompilePath)
+    precompile_deactivator(packagePath, precompilePath)
 
 Deactivates precompile of a package by commenting include() of *.jl file generated by SnoopCompile and _precompile_().
 
+packagePath is the same as `pathof`. However, `pathof(module)` isn't used to prevent loadnig the package.
+
 Written exclusively for SnoopCompile Github actions.
 """
-function precompileDeactivator(packagePath::String, precompilePath::String)
+function precompile_deactivator(packagePath::String, precompilePath::String)
 
-    file = open(packagePath,"r")
-    packageText = Base.read(file, String)
-    close(file)
+    packageText = Base.read(packagePath, String)
 
-    c1, c2, a1, a2 = precompileRegex(precompilePath)
+    c1, c2, a1, a2 = precompile_regex(precompilePath)
 
     # Checking availability of _precompile_ code
     commented = occursin(c1, packageText) && occursin(c2, packageText)
@@ -102,9 +101,8 @@ function precompileDeactivator(packagePath::String, precompilePath::String)
                      ),
                      init = packageText)
 
-                    file = open(packagePath,"w")
-                    Base.write(file, packageEdited)
-                    close(file)
+                     Base.write(packagePath, packageEdited)
+
         println("precompile is deactivated")
     elseif commented
         # do nothing