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

little changes #8410

Merged
merged 3 commits into from Jan 16, 2016
Merged

little changes #8410

merged 3 commits into from Jan 16, 2016

Conversation

latot
Copy link
Contributor

@latot latot commented Jan 9, 2016

Hi, well this would be greate if some one can check this please.

@hrydgard
Copy link
Owner

hrydgard commented Jan 9, 2016

preincrement used to be slightly faster for iterators on older C++ compilers, but it certainly doesn't make a difference on integer types, and I don't think it makes a difference any more at all anyway. So I'm not going to merge that part.

@latot
Copy link
Contributor Author

latot commented Jan 9, 2016

okis, removed

back 1

other little change, join 2 in 1
@latot latot force-pushed the mini branch 2 times, most recently from 71ad9a1 to 53ac6d9 Compare January 10, 2016 01:20
@latot
Copy link
Contributor Author

latot commented Jan 10, 2016

to avoid problem with compilations in forks...

@latot
Copy link
Contributor Author

latot commented Jan 10, 2016

@hrydgard a little question, in this lines of Common/ArmEmitter.cpp:902

    if (op == -1)
        _assert_msg_(JIT, false, "%s not yet support %d", InstNames[Op], Rm.GetType()); 
    Write32(condition | (op << 21) | (SetFlags ? (1 << 20) : 0) | Rn << 16 | Rd << 12 | Data);

whats the posibility of the vars op or Rn or Rd take a negative value?

@Bigpet
Copy link
Collaborator

Bigpet commented Jan 10, 2016

@latot please buddy. Disable the travis irc bot when you enable travis on your fork
image

@hrydgard
Copy link
Owner

@latot shouldn't happen really.

Also as bigpet says, you need to look at your travis chatbot config..

@latot
Copy link
Contributor Author

latot commented Jan 10, 2016

mm, this is weird, i disabled the fork from travis D:
or may be are the messages before change it (i resync the fork so the .travis.yml reset too), well the commit of travis is to fix this behavior betwen the main project and forks.

its posible merge this now? or need others changes?

@@ -1,5 +1,7 @@
# https://travis-ci.org

sudo: required
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

travis do some changes the some year, the new proyect added after 2014, need include this line to works if it requieres root, in the proyect don't affects this but forks yes, and is very problematic when at least i try to make a pull request, any pr fails but if i do a change to can works with travis but that commit will be reflacted in the pr, finally i think its better inlcude it and simplify this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that was part of the whitelisting we got earlier, because it was before/around when they added the containers.

I suppose this isn't an issue, though.

-[Unknown]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mm, well its right don't is a issdue of the main proyects, but as we saw with all my failed commits (resync fork) in the irc affects the forks, anyway i only can opine about this, then remove or not?

remove
@latot
Copy link
Contributor Author

latot commented Jan 16, 2016

next time i'll remove it directly, sorry the problems i'm very unsecure with that.

hrydgard added a commit that referenced this pull request Jan 16, 2016
@hrydgard hrydgard merged commit 067095e into hrydgard:master Jan 16, 2016
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.

None yet

4 participants