Skip to content
This repository has been archived by the owner on Dec 6, 2019. It is now read-only.

Dev #112

Closed
wants to merge 15 commits into from
Closed

Dev #112

wants to merge 15 commits into from

Conversation

Oxymoron290
Copy link

Resolution for issue #5
Resolution for issue #110
Resolution for issue #111

@Oxymoron290
Copy link
Author

is there an issue with my pull request?

@nabeelio
Copy link
Member

Sorry, I'm reviewing it right now.

|| $this->post->depicao == '' || $this->post->arricao == ''
|| $this->post->aircraft == '' || $this->post->flighttime == ''
public function edit_pirep_post($postData = null) {
$postData = ($postData == null) ? $this->post : $postData;
Copy link
Member

Choose a reason for hiding this comment

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

Why the change to $postData from $this->post?

Copy link
Author

Choose a reason for hiding this comment

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

I believe this was done during a mass find/replace. It's been a while but I don't believe it changed functionality. Had more to do with keeping things uniform

Copy link
Author

Choose a reason for hiding this comment

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

Ah! I remember, I added a default parameter, so this could be called from another function without using the Vars class. Adding API functionality and all that jazz.

@Oxymoron290
Copy link
Author

And the verdict is?...

@aarbeee
Copy link

aarbeee commented Apr 18, 2014

nshahzad,
Could you please have a look in to the question?
It seems that I have an issue since I am over to a new hosting company and that the solution is in this package.

Sincere,

RobB

@nabeelio
Copy link
Member

Hi @Oxymoron290, sorry for the super long delay. Are these commits still OK to merge in?

@Oxymoron290
Copy link
Author

Yes, ensure the last commit is 775625b as it was a conflict resolution. No worries on the delay, we all have priorities.

@nabeelio
Copy link
Member

I made a couple of comments, if you can look at those, I will merge it. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants