Skip to content

Logger: Add possibility to send sync requests. - #6356

Merged
polesye merged 1 commit into
masterfrom
anton/logger
Jan 15, 2015
Merged

Logger: Add possibility to send sync requests.#6356
polesye merged 1 commit into
masterfrom
anton/logger

Conversation

@polesye

@polesye polesye commented Dec 23, 2014

Copy link
Copy Markdown
Contributor

In this PR was done:

  • *.coffee -> *.js;
  • added possibility to set request parameters;
  • fixed and added new jasmine tests;
  • removed unnecessary Courseware.prefix.

@mulby, @andy-armstrong please review.

andy-armstrong
andy-armstrong reviewed Dec 23, 2014
View reviewed changes
Comment thread common/static/js/spec/logger_spec.js Outdated

@andy-armstrong andy-armstrong Dec 23, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo: should be 'can listen to events...'

@polesye polesye Dec 23, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Typo: should be 'can listen to events...'

Fixed.

@andy-armstrong

andy-armstrong commented Dec 23, 2014

Copy link
Copy Markdown
Contributor

👍 Looks good to me if you catch exceptions from the callbacks.

@jmclaus jmclaus mentioned this pull request Dec 23, 2014
Closed
@polesye

polesye commented Dec 24, 2014

Copy link
Copy Markdown
Contributor Author

@mulby please review.

@mulby

mulby commented Dec 24, 2014

Copy link
Copy Markdown
Contributor

I'm on vacation until Monday.
On Dec 24, 2014 11:04 AM, "Anton Stupak" notifications@github.com wrote:

@mulby https://github.com/mulby please review.

—
Reply to this email directly or view it on GitHub
https://github.com/edx/edx-platform/pull/6356#issuecomment-68060647.

mulby
mulby reviewed Dec 29, 2014
View reviewed changes
Comment thread common/static/js/src/logger.js Outdated

@mulby mulby Dec 29, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this simply the result of compiling the coffee script file?

@polesye polesye Dec 29, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this simply the result of compiling the coffee script file?

No, it was rewritten by me. Missing tests were added.

@mulby

mulby commented Dec 29, 2014

Copy link
Copy Markdown
Contributor

Generally looks good, I posted a few high level questions.

@polesye

polesye commented Jan 12, 2015

Copy link
Copy Markdown
Contributor Author

@mulby Do you agree with merging this PR?

@polesye

polesye commented Jan 14, 2015

Copy link
Copy Markdown
Contributor Author

If no objection I'll merge it once Jenkins pass.

polesye added a commit that referenced this pull request Jan 15, 2015
@polesye
Merge pull request #6356 from edx/anton/logger
477031e 
Logger: Add possibility to send sync requests.
@polesye
polesye merged commit 477031e into master Jan 15, 2015
@polesye
polesye deleted the anton/logger branch January 15, 2015 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@polesye @andy-armstrong @mulby @cpennington @singingwolfboy