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

Changed issym to issymmetric. #15192

Merged
merged 1 commit into from
Feb 28, 2016
Merged

Conversation

pkofod
Copy link
Contributor

@pkofod pkofod commented Feb 22, 2016

Fixes #15182 .

grepping /julia/ tells me that there is an issym variable in some .c-files left, but they should obviously not be changed (they are boolean variables asking if a variable is a symbol by the way, so that strengthens the need for being precise).

There is something I still need to do. grep -r issym still tells me (relevant) issym's still live in
contrib/BBEditTextWrangler-julia.plist
base/docs/helpdb/Base.jl
doc/stdlib/linalg.rst

I am unsure how the documentation system works in julia. Should I just change these files as well, or are some things auto-generated, and others are not - or? Thanks.

Edit: Obviously also need to deprecate issym, will do in the morning

@andreasnoack
Copy link
Member

Regarding the documentation then check out https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md#improving-documentation. However, it can be a little confusing so feel free to ask.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 23, 2016

Thanks for the pointer. I hope I did it correctly, let's see if it builds on the CI-services. I could generate the docs locally without errors at least.

I also @deprecated issym issymmetric.

@pkofod pkofod force-pushed the pkm/issymmetric branch 2 times, most recently from b22a89b to 5b0f497 Compare February 23, 2016 10:41
@pkofod
Copy link
Contributor Author

pkofod commented Feb 23, 2016

I had missed an issym in linalg/dense.jl, but only the linux builds failed on travis, the OSX builts were OK. Is that supposed to be possible? https://travis-ci.org/JuliaLang/julia/builds/111159068

@tkelman
Copy link
Contributor

tkelman commented Feb 23, 2016

OSX Travis is not currently running all tests to avoid timing out.

This looks good, thanks! I like git grep for this kind of renaming check.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 23, 2016

Sounds like i could have benefitted from git grep. Cool, will use that next time.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 25, 2016

Are there others places to correct the change than the docs and @deprecateing the old name?

@tkelman
Copy link
Contributor

tkelman commented Feb 25, 2016

Worth adding a note about the renaming to NEWS.md perhaps?

@pkofod
Copy link
Contributor Author

pkofod commented Feb 25, 2016

I've added a bullet. I chose favor in favour of favour, but both seem to be used throughout NEWS.md.

@pkofod pkofod force-pushed the pkm/issymmetric branch 2 times, most recently from eec25cf to 57d6084 Compare February 25, 2016 08:36
@pkofod
Copy link
Contributor Author

pkofod commented Feb 25, 2016

I force pushed the change, so unfortunately your message is gone, but I've moved it to the correct section.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 25, 2016

Sorry, I actually remember having read that you should do that when adding something to NEWS.md. Thanks.

@andreasnoack
Copy link
Member

@pkofod Could you rebase this?

@pkofod
Copy link
Contributor Author

pkofod commented Feb 26, 2016

@andreasnoack I will fix it later, but yes.

I guess I just need to reorder and squash the commits, right?
Edit: i think i know what i should have done. Oh well, ill fix it.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 26, 2016

I hate and love git. Kind of messed the rebase up, but was able to go back, and it should be 👍 now

@pkofod
Copy link
Contributor Author

pkofod commented Feb 27, 2016

What on earth did I do to make it build all the dependencies and fail? Did I err, or is it a travis thing?

@tkelman
Copy link
Contributor

tkelman commented Feb 27, 2016

The Travis caching for dependencies was in a messed-up state, ref #15249, but it's fixed now - restarted.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 27, 2016

Thanks!

andreasnoack added a commit that referenced this pull request Feb 28, 2016
@andreasnoack andreasnoack merged commit d8d1489 into JuliaLang:master Feb 28, 2016
@andreasnoack
Copy link
Member

@pkofod Thanks. It would be great if you could also add an entry to Compat.jl for issymmetric.

@KristofferC
Copy link
Member

JuliaLang/Compat.jl#175 for a similar PR to compat.

@pkofod
Copy link
Contributor Author

pkofod commented Feb 28, 2016

Sure, I'll do it later!

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.

4 participants