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

Added Student Mastery/Class Mastery CoachReports #3033

Merged
merged 8 commits into from
Feb 20, 2015

Conversation

anuragkanungo
Copy link
Contributor

No description provided.

@anuragkanungo
Copy link
Contributor Author

@aronasorman
I haven't done re factoring yet.

@anuragkanungo
Copy link
Contributor Author

@aronasorman
Get that reviewed asap.

@aronasorman
Copy link
Collaborator

yessir!

@anuragkanungo
Copy link
Contributor Author

Haha, Thanks 👍

@aronasorman aronasorman assigned jamalex and unassigned aronasorman Feb 12, 2015
@aronasorman aronasorman added this to the Nalanda bugfixes milestone Feb 12, 2015
@jamalex
Copy link
Member

jamalex commented Feb 12, 2015

When opening a PR like this, can you please include a description with explanations of what it does, and screenshots of changes made?

@jamalex
Copy link
Member

jamalex commented Feb 12, 2015

This is going to have substantial merge conflicts with the develop branch (see anuragkanungo/ka-lite@learningequality:develop...anuragkanungo:coach_reports).

Are we aiming to abandon the nalanda branches moving forward (i.e. not aim to merge them to develop), and just cherry-pick needed pieces from there into the develop branch? @rtibbles @aronasorman

@aronasorman
Copy link
Collaborator

I think it makes sense to just complete drop it and then merge piecemeal.

We can revisit the PRs merged into nalanda by looking at the nalanda-bugfixes milestone, and then porting whatever PRs are relevant to the develop branch.

@anuragkanungo
Copy link
Contributor Author

screenshot from 2015-02-12 16 33 17

@jamalex
Copy link
Member

jamalex commented Feb 12, 2015

@anuragkanungo We can review and merge this in, as long as it's understood that this (and the nalanda-* branches in general) won't be getting merged back into the develop branch anymore, as they are diverging too much, and hence any features/fixes that are intended to outlive the RCT will need to be cherry-picked (or redone) onto the develop branch directly (where they will need to pass the "does this belong in the mainline release" criterion, and hence, unlike the process for this PR, should be discussed in detail in an issue, prior to being worked on).

@@ -0,0 +1,97 @@
/**

This comment was marked as spam.

This comment was marked as spam.

@jamalex
Copy link
Member

jamalex commented Feb 12, 2015

Given that (as discussed above) this code is temporary (constrained to the nalanda-* branches), I'd be ok merging it as-is, but made some comments regarding the kinds of things that would need to be changed if we were doing this against the mainline branches.

@anuragkanungo
Copy link
Contributor Author

@jamalex , I did most the changes as you suggested.

@@ -0,0 +1,97 @@
/**

This comment was marked as spam.

This comment was marked as spam.

jamalex added a commit that referenced this pull request Feb 20, 2015
Added Student Mastery/Class Mastery CoachReports
@jamalex jamalex merged commit 8e5e8e5 into learningequality:nalanda-rct3 Feb 20, 2015
@jamalex jamalex removed the has PR label Feb 20, 2015
@benjaoming benjaoming mentioned this pull request Jul 16, 2019
2 tasks
benjaoming pushed a commit to benjaoming/ka-lite that referenced this pull request Jul 16, 2019
benjaoming pushed a commit to benjaoming/ka-lite that referenced this pull request Jul 16, 2019
benjaoming pushed a commit to benjaoming/ka-lite that referenced this pull request Jul 16, 2019
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.

3 participants