Skip to content

Fix #294: make callRemote return 0 as errno upon successful request #295

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Sep 2, 2019
Merged

Fix #294: make callRemote return 0 as errno upon successful request #295

merged 5 commits into from
Sep 2, 2019

Conversation

@patrikjuvonen
Copy link
Contributor

@patrikjuvonen patrikjuvonen commented Aug 3, 2018 •
edited
Loading

GitHub issue:
This PR closes #294

Summary:

  • Update callRemote to return errno 0 instead of nil. Improves consistency between fetchRemote and callRemote.
  • Main question is about backwards compatibility. What should we do about it?

@patrikjuvonen patrikjuvonen added the bug Something isn't working label Aug 3, 2018
@patrikjuvonen patrikjuvonen added this to the 1.5.6 milestone Aug 3, 2018
@qaisjp
Copy link
Contributor

qaisjp commented Aug 29, 2018

Dropping from 1.5.6 as this is something we will want to put in a nightly for testing

@qaisjp qaisjp removed this from the 1.5.6 milestone Aug 29, 2018
@patrikjuvonen patrikjuvonen added this to the 1.5.7 milestone Sep 4, 2018
@qaisjp qaisjp changed the base branch from master to next September 4, 2018 23:58
@qaisjp qaisjp changed the base branch from next to master September 4, 2018 23:58
@qaisjp qaisjp requested a review from jushar September 4, 2018 23:59
@qaisjp
Copy link
Contributor

qaisjp commented Sep 4, 2018

Requesting feedback from @Jusonex as I believe you've mentioned something about this once? (?)

@patrikjuvonen patrikjuvonen added the feedback Further information is requested label Sep 5, 2018
@qaisjp qaisjp removed this from the 1.5.7 milestone Sep 7, 2018
@jushar
Copy link
Contributor

jushar commented Sep 8, 2018 •
edited
Loading

As my feedback was requested:
I cannot estimate the risk of breaking scripts. I'd be fine by merging it and checking whether or not people complain about problems. At the utmost there are maybe a handful of people affected by this, maybe not.

Perhaps it makes sense to announce the possible incompatibility on Discord, if we decide to merge it.

@patrikjuvonen patrikjuvonen removed the feedback Further information is requested label Sep 8, 2018
@patrikjuvonen patrikjuvonen changed the title #294: make callRemote return 0 as errno upon successful request Fix #294: make callRemote return 0 as errno upon successful request Feb 24, 2019
@botder botder added this to the Backlog milestone Mar 4, 2019
@patrikjuvonen patrikjuvonen changed the title Fix #294: make callRemote return 0 as errno upon successful request [1.6] Fix #294: make callRemote return 0 as errno upon successful request May 19, 2019
@patrikjuvonen patrikjuvonen modified the milestones: Backlog, 1.6 May 19, 2019
@patrikjuvonen patrikjuvonen changed the title [1.6] Fix #294: make callRemote return 0 as errno upon successful request Fix #294: make callRemote return 0 as errno upon successful request May 20, 2019
@qaisjp
Copy link
Contributor

qaisjp commented Jul 18, 2019 •
edited
Loading

Proposal:

  • announce change in return value during 1.5.7 release
  • include some other INFO message in scripts?
  • merge this after 1.5.7 has released and observe complaints amongst nightly users
  • release change in 1.6

@patrikjuvonen
Copy link
Contributor Author

patrikjuvonen commented Jul 20, 2019

Proposal:

  • announce change in return value during 1.5.7 release
  • include some other INFO message in scripts?
  • merge this after 1.5.7 has released and observe complaints amongst nightly users
  • release change in 1.6

I think this sounds fine.

@qaisjp qaisjp modified the milestones: 1.6, 1.5.7 Jul 21, 2019
@qaisjp
Copy link
Contributor

qaisjp commented Jul 21, 2019 •
edited
Loading

Ok, unless anyone else feels differently, let's go with that.

I've set the milestone to 1.5.7 so that we don't forget to announce the change / consider the INFO message before we release 1.5.7.

DO NOT MERGE THIS PR IN 1.5.7!

(maybe we should move this proposal to #294 & milestone that instead?)

Edit 8th August: I've milestoned this as 1.6 now that we have the "Early announcement of 1.6 changes" issue

@qaisjp qaisjp mentioned this pull request Aug 7, 2019
Closed
5 tasks
@qaisjp qaisjp modified the milestones: 1.5.7, 1.6 Aug 8, 2019
qaisjp added a commit that referenced this pull request Aug 17, 2019
@qaisjp
Add callRemote warning for #294, #295 and #1044
a8c41df 
The backspaces is suppresses the following text from the deprecation
message: "no longer works"

This results in a message like "callRemote callbacks will [..]"
qaisjp added a commit that referenced this pull request Aug 28, 2019
@qaisjp
Revert "Add callRemote warning for #294, #295 and #1044"
f2407e7 
Either needs removing or rewording

1. not clear enough
2. backspace characters only work in terminal, not for logs, obviously

This reverts commit a8c41df.
@qaisjp qaisjp merged commit a914a58 into multitheftauto:master Sep 2, 2019
qaisjp added a commit that referenced this pull request Sep 2, 2019
@qaisjp
Revert "Revert "CRemoteCalls: make callRemote return 0 as errno upon …
507de5f 
…success (#295)""

This reverts commit 25ffe62.
qaisjp added a commit that referenced this pull request Sep 2, 2019
@qaisjp
Revert "CRemoteCalls: make callRemote return 0 as errno upon success (#…
25ffe62 
…295)"

This reverts commit a914a58.
@patrikjuvonen patrikjuvonen deleted the github-issue-294 branch August 5, 2021 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jushar jushar Awaiting requested review from jushar

1 more reviewer

@qaisjp qaisjp qaisjp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@qaisjp qaisjp

Labels

bug Something isn't working

Projects

None yet

Milestone

1.6.0

Development

Successfully merging this pull request may close these issues.

callRemote will always return a nil errno even if successful

4 participants

@patrikjuvonen @qaisjp @jushar @botder