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

Implement Object.property.hasOwnProperty #41

Conversation

galpeter
Copy link
Contributor

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com

@galpeter galpeter force-pushed the object_prototype_hasownproperty branch from 26c82fa to 5c2e159 Compare May 13, 2015 12:30
? ECMA_SIMPLE_VALUE_TRUE
: ECMA_SIMPLE_VALUE_FALSE);
}
ECMA_FINALIZE (obj_val)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, add semicolon after statement.

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com
@galpeter galpeter force-pushed the object_prototype_hasownproperty branch from 5c2e159 to 2f2e811 Compare May 15, 2015 08:57
@galpeter
Copy link
Contributor Author

I've updated the pull request.

@egavrin
Copy link
Contributor

egavrin commented May 15, 2015

Looks good. make push

@galpeter
Copy link
Contributor Author

I've got a forked repository with that the make push will not work as expected. Can't I just push the Merge pull request button?

@ruben-ayrapetyan
Copy link
Contributor

@galpeter , please do the following on the repository:

git checkout -b galpeter-object_prototype_hasownproperty master
git pull https://github.com/galpeter/jerryscript.git object_prototype_hasownproperty
git rebase master
git checkout master
git merge --ff-only galpeter-object_prototype_hasownproperty
make push

@ruben-ayrapetyan
Copy link
Contributor

Looks like the sequence could be put into some script in tools directory.
Evgeny, what do you think about the idea?

@egavrin
Copy link
Contributor

egavrin commented May 15, 2015

@ruben-ayrapetyan I think this is not needed, wiki page for that would be OK.

@galpeter
Copy link
Contributor Author

merged: 6c4e988

@galpeter galpeter closed this May 15, 2015
@galpeter
Copy link
Contributor Author

@ruben-ayrapetyan, @egavrin

Using the rebase during the merge, will result a different hash, thus the after pushing the merge (well rebase in terms) the github will not detect that the given pull request was merged and the person who did the merge needs to go to the issue and close it manually (and probably enter the link to the merge hash). This is a waste of time IMHO.

@ruben-ayrapetyan
Copy link
Contributor

@galpeter , try to use push -f for updating your branch.
We performed this for pull requests, and they were marked as merged automatically.

@galpeter
Copy link
Contributor Author

@ruben, sure I can do that but I need to do it on the other (forked) repo (as this is currently a forked pull-request) and it's a huge waste of time to rebase the branch first, upload it, then go to the other repo pull down the, perform again the steps described above (hope that the upstream didn't had any new commits since then, if it did have then the rebase will change the hash) and do the make push (and again, hope that during the make push's precommit check there was no new commits in the master).

@ruben-ayrapetyan
Copy link
Contributor

@galpeter ,
Yes, it really seems to be inconvenient in case of forked repository.
Hope, soon we would use branch model, and the procedure would be ok.
I used it for all of my pull requests without any issues rebasing branch to master.

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

3 participants