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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Finishing up Project.toml configuration #333

Closed
wants to merge 11 commits into from
Closed

Finishing up Project.toml configuration #333

wants to merge 11 commits into from

Conversation

tkf
Copy link
Contributor

@tkf tkf commented Jul 20, 2018 •
edited

For SIMD tests in #332 (comment) to work, I thought the easiest solution would be to just add Project.toml now that ForwardDiff.jl dropped 0.6 support in #329. There are a few things to do finalize Pkg3 support. I want to keep #332 simple so I thought to make a new PR on top of it.

Here are a few TODOs:

  • Add compatibility version numbers in [compat] section.
  • Decide what to put in version. I go ahead and put version = "0.8.0". Is that OK?
  • Add authors in Project.toml. I'm totally new to this project so I don't want to be the one to add it 馃槃 Please feel free to edit/extend this PR and finishing it.

@tkf tkf mentioned this pull request Jul 20, 2018
Closed
@tkf tkf changed the title Migration to Pkg3 Finishing up Project.toml configuration Jul 20, 2018
tkf added 6 commits July 20, 2018 14:56
@jrevels jrevels requested a review from KristofferC July 22, 2018 20:49
@jrevels
Copy link
Member

jrevels commented Jul 23, 2018

I go ahead and put version = "0.8.0". Is that OK?

I've just tagged v0.8.0 in response to requests to have a v0.7-compatible ForwardDiff release (JuliaLang/METADATA.jl#16019), so presumably this PR would now make it in ForwardDiff v0.8.1.

I've requested @KristofferC's review here since I'm pretty unfamiliar with the configuration format for the new package manager.

Just to clarify - AFAICT, it seems like this PR just replaces #332 entirely. Is that correct? Alternatively, you could base this PR against the branch for #332 (tkf:literal_pow) instead of master. I'm fine either way, I just want to make sure I understand what's going on 馃檪

@tkf
Copy link
Contributor Author

tkf commented Jul 23, 2018

it seems like this PR just replaces #332 entirely. Is that correct?

Yes. This PR extends the branch of #332. I was thinking to rebase this branch on top of master once #332 was merged but merging this PR directly is also fine. I can (fast-forward) merge this to #332 if you want to do the review there.

Alternatively, you could base this PR against the branch for #332 (tkf:literal_pow) instead of master

I thought I can do that but it looks like PR goes to my repository instead of here: tkf#1

My intention was to simplify the review of #332 to include Project.toml just enough to make SIMD tests pass. Also, at first it wasn't clear how to make tests work in AppVeyor (which is resolved now) so I thought it may take sometime to finish up this PR. I opened a separate PR just for your convenience (not to confuse you 馃槈) so please choose whichever you like to review/merge.

FYI these are the addition in this PR to #332:
tkf/ForwardDiff.jl@literal_pow...pkg3

@tkf tkf mentioned this pull request Jul 27, 2018
Merged
@tkf
Copy link
Contributor Author

tkf commented Jul 29, 2018

Closing this for now, as we don't include Project.toml in #332 (#332 (comment)).

@tkf tkf closed this Jul 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KristofferC KristofferC Awaiting requested review from KristofferC

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@tkf @jrevels