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

core/issues/96 - Expose source column in booking report #12073

Closed
wants to merge 1 commit into from

Conversation

yashodha
Copy link
Contributor

@yashodha yashodha commented May 2, 2018

Overview

Expose source column in booking report

Before

book_before

After

book_after

@@ -301,6 +301,9 @@ public function __construct() {
'title' => ts('Contribution Status'),
'default' => TRUE,
),
'source' => array(
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to use the same key as below & define a 'name' as below?

@eileenmcnaughton
Copy link
Contributor

Makes sense - one minor comment inline

@eileenmcnaughton
Copy link
Contributor

@yashodha @nganivet just a minor thing to get this mergeable - but I notice there are a few PRs you have raised that have unanswered reviewer feedback blocking them being merged. We should figure out which ones you want to follow up on & close the others

@mattwire
Copy link
Contributor

@yashodha Could you update the PR subject per https://docs.civicrm.org/dev/en/latest/tools/git/#pr-subject:
"dev/core#96 - Expose source column in booking report"

@eileenmcnaughton
Copy link
Contributor

@yashodha @nganivet I'm going to close this & let you re-open it when the reviewer feedback is addressed. There is nothing lost by closing a PR as it can easily be re-opened and there is a real time cost in reviewers check in on a PR only to find it is not reviewable (e.g because a change needs to be made or the PR requires a unit test).

If you need a way to track changes you are working on getting upstreamed & you want to use something other than your own issue tracker then gitlab is a good option

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

Successfully merging this pull request may close these issues.

4 participants