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

Fix part of #6584: Modified inline message below Title and Goal textbox #6596

Closed
wants to merge 7 commits into from

Conversation

Projects
None yet
4 participants
@ithompson4
Copy link

commented Apr 13, 2019

Explanation

Fix part of #6584: Modified inline messages below Title textbox and Goal textbook with 15 minimum characters required.

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR follows the style guide.
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.
@oppiabot

This comment has been minimized.

Copy link

commented Apr 13, 2019

Hi! @ithompson4. Welcome to Oppia! Please could you follow the instructions here to get started? You'll need to do this before we can accept your PR. Thanks!

@ithompson4 ithompson4 changed the title Fix part of #6584: Modified inline message below Title textbox and Go… Fix part of #6584: Modified inline message below Title and Goal textbox Apr 13, 2019

@ithompson4 ithompson4 closed this Apr 13, 2019

@ithompson4 ithompson4 reopened this Apr 13, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Apr 13, 2019

Codecov Report

Merging #6596 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6596   +/-   ##
========================================
  Coverage    75.99%   75.99%           
========================================
  Files          999      999           
  Lines        79409    79409           
  Branches      4703     4703           
========================================
  Hits         60345    60345           
  Misses       19064    19064

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fb2544...0cc5890. Read the comment docs.

@anubhavsinha98
Copy link
Member

left a comment

Hi @ithompson4, you should not delete the earlier message. Rather add the Minimum 15 characters required message with the existing ones. If you have any doubt feel free to ask, Thanks! :)

Iryna Thompson Iryna Thompson
@oppiabot

This comment has been minimized.

Copy link

commented Apr 17, 2019

Hi @ithompson4. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Hi @ithompson4 I think you just need to resolve the conflict and look at the comment above. Then the PR is good to go!
Thanks!

Iryna Thompson Iryna Thompson
@ithompson4

This comment has been minimized.

Copy link
Author

commented Apr 18, 2019

To resolve this merge conflict, do I need to get the latest code for my branch, open the file with conflicts, and remove unnecessary code? Not done this before so please advise. Thanks.

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

To resolve this merge conflict, do I need to get the latest code for my branch, open the file with conflicts, and remove unnecessary code? Not done this before so please advise. Thanks.

Hi @ithompson4, first of try to pull the branch by using the git pull origin branchname and then look for the files in which the merge conflicts occurs. Then try to resolve them.

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

Also take a look at this article I hope it will also help https://www.atlassian.com/git/tutorials/using-branches/merge-conflicts . If not feel free to ask. Thanks!

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

Hi, @ithompson4 can you post the screenshot of the UI how its look after the changes?

@ithompson4

This comment has been minimized.

Copy link
Author

commented Apr 19, 2019

The UI screenshot:
textBoxMsg

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

Hi @ithompson4 can you update your branch with develop?

@ithompson4

This comment has been minimized.

Copy link
Author

commented Apr 20, 2019

Hello @anubhavsinha98. I tried to update my working branch with develop and I got this error:

Irynas-MacBook-Pro:oppia irynathompson$ git fetch upstream
Irynas-MacBook-Pro:oppia irynathompson$ git merge upstream/develop
Already up to date.
Irynas-MacBook-Pro:oppia irynathompson$ git status
On branch issue-6584
nothing to commit, working tree clean
Irynas-MacBook-Pro:oppia irynathompson$ git push origin issue-6584
Warning: Please set upstream for the lint checks to run efficiently. You can learn more about it here -> https://git-scm.com/book/en/v2/Git-Branching-Remote-Branches

To https://github.com/ithompson4/oppia.git
 ! [rejected]            issue-6584 -> issue-6584 (non-fast-forward)
error: failed to push some refs to 'https://github.com/ithompson4/oppia.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

How can I resolve this problem? Thanks.

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

Hi @ithompson4, the PR looks good!
I think to resolve the above issue, try to git pull origin branchname. If the error still occurs feel free to ask!

Iryna Thompson added some commits Apr 23, 2019

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

Thanks, @ithompson4 for updating the branch.
Also @DubeySandeep can you review this PR!
Thanks :)

@DubeySandeep

This comment has been minimized.

Copy link
Member

commented May 1, 2019

@ithompson4, Sorry for being late here! I had a discussion with other team members regarding the requirement of this PR and we think we don't need any such message to explain the min. the requirement for the goal. I think a meaningful goal automatically have one that's more than 15 chars and we don't want people to write anything just to fulfill the 15 char requirements.

@anubhavsinha98, can you please help @ithompson4 to find a similar issue?

Sorry for this decision, do let me know if I can help you out anyway!

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented May 1, 2019

Hi @ithompson4, can you find any other issue to work upon. I would surely help you out to resolve that issue.
Thanks!

@anubhavsinha98

This comment has been minimized.

Copy link
Member

commented May 1, 2019

Hi @ithompson4, I think this issue which is regarding the backend test will be a good one to start.
#6550
Or you can go for #6387, which is regarding remaining GLOBALS from HTML.
Do let me know, if you want me to suggest more issues, I will be happy to help.
Thanks :)

@DubeySandeep

This comment has been minimized.

Copy link
Member

commented May 8, 2019

@ithompson4 and @anubhavsinha98, I'm closing this PR (as discussed in the email)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.