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

Bot for Github actions #49

Merged
merged 40 commits into from
Jan 14, 2020
Merged

Bot for Github actions #49

merged 40 commits into from
Jan 14, 2020

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Dec 6, 2019

Adds support for #37

These are some utilities that I will be using for the SnoopCompile bot. I think the best place for these functions is the SnoopCompile itself.

@aminya aminya force-pushed the packageSnooper branch 2 times, most recently from 65090bd to 52a2d78 Compare December 12, 2019 06:55
@aminya aminya force-pushed the packageSnooper branch 3 times, most recently from b52f3ba to 0f3cd46 Compare December 12, 2019 09:39
@aminya

This comment has been minimized.

@aminya

This comment has been minimized.

@aminya

This comment has been minimized.

aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 2, 2020
aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 2, 2020
aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 5, 2020
aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 5, 2020
aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 5, 2020
aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 5, 2020
@aminya
Copy link
Contributor Author

aminya commented Jan 5, 2020

@KristofferC @timholy response to a couple of comments:
#49 (comment)
#49 (comment)
#49 (comment)

I am assuming that the users of this package are following the rules I set and specified in the documentation, and so the code works for these situations.

Handling all the other possibilities does not have much value.


For example:

To make the whole code independent of the current working directory and where packages files are placed a couple of things should be done.
For example to get the path of the package we should do:

packagePath = Base.read(`cmd /c julia -e 'import MatLang; print(pathof(MatLang))'`, String)

instead of https://github.com/aminya/SnoopCompile.jl/blob/32d36d1e9a2bc3d22fa90afe3652e2f404689e5d/src/bot/snoopiBot.jl#L30

packagePath = joinpath(pwd(),"src","$packageName.jl")

or for precompile_$packageName.jl, some script can search in the package folder.


or if the user decides to have multiple precompile inclusion by mistake, such as:

# include("precompile.jl")
# _precompile_()

# some other stuff

include("precompile.jl")
_precompile_()

The code will not work.

aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 6, 2020
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)
@aminya
Copy link
Contributor Author

aminya commented Jan 6, 2020

I squashed the commits that addressed review comments. If you approve, this can be merged.

I added two issues to not forget the remaining comments once this is merged.
#62
#61

aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 6, 2020
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
Copy link
Contributor Author

aminya commented Jan 6, 2020

Are you planning on maintaining this code long-term? I have made a suggestion to mark it as "EXPERIMENTAL" because I already maintain too much code and do not want to be on the hook for this.

I can maintain the code. Should I remove "EXPERIMENTAL"?

aminya added a commit to aminya/SnoopCompile.jl that referenced this pull request Jan 6, 2020
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)
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)

accurate error message
timholy#49 (comment)
@aminya
Copy link
Contributor Author

aminya commented Jan 12, 2020

@timholy This is ready. Should I do anything else?

@aminya aminya requested a review from timholy January 12, 2020 22:01
@timholy
Copy link
Owner

timholy commented Jan 14, 2020

Let's do it! Thanks for a huge contribution!

I can maintain the code. Should I remove "EXPERIMENTAL"?

Let's leave it as an insurance policy.

@timholy timholy merged commit d515bc6 into timholy:master Jan 14, 2020
@timholy
Copy link
Owner

timholy commented Jan 14, 2020

Version 1.2 is out: JuliaRegistries/General#7894

Care to do the honors of writing a discourse post?

@aminya
Copy link
Contributor Author

aminya commented Jan 14, 2020

Version 1.2 is out: JuliaRegistries/General#7894

Care to do the honors of writing a discourse post?

Awesome! I would be happy to write a post 🚀

@aminya aminya changed the title Utilities for SnoopCompile Github actions Bot for Github actions Jan 15, 2020
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.

3 participants