Fixes #1856 - Better error messaging for unset milestones

[marimeireles]

Hi @magsout, I'm not sure if I should ping you or Karl or someone else, so please let me know if I'm pinging the wrong person.

Sorry for the delay I got caught up with school these last few days.
Do you think it's a good solution? Also, did I get the variable names right this time?

Thanks! :)

[marimeireles]

I just saw it failed on the CI tests, this was the test it failed:

× firefox on linux 4.4.0-137-generic - Milestones (non-auth) - Missing status error displays (0.201s)
    AssertionError: expected 'No status assigned yet' to equal 'Error: no status for this issue'
    
    A No status assigned yet
    E Error: no status for this issue
    
      at Command.<anonymous>  <tests/functional/milestones-non-auth.js:43:18>
      at Test.Missing status error displays [as test]  <tests/functional/milestones-non-auth.js:41:10>
      at <src/lib/Test.ts:263:51>

Should I change this test so it works with the new strings? If yes, should I do this on this issue or should I open a new one?
Thanks!

[karlcow]

Should I change this test so it works with the new strings? If yes, should I do this on this issue or should I open a new one?

@marimeireles Fix the test on this issue. :) That's part of it. The test worked by flagging the change.

[Anushi1998]

@marimeireles Also it would be great if you can put # before issue number as Git cannot link it now :)

[karlcow]

@marimeireles @Anushi1998 The style we use for commits messages (Step 5) is described in the docs and mildly enforced (when I do not forget).

[magsout]

@marimeireles

Hi @magsout, I'm not sure if I should ping you or Karl or someone else, so please let me know if I'm pinging the wrong person.

it depends of the stuff. @karlcow or @miketaylr are the specialist of python
@miketaylr and me, are more about JS or CSS stuff. You can ping many people if needed

[marimeireles]

Fix the test on this issue. :) That's part of it. The test worked by flagging the change.

Got it! Thanks, @karlcow. :)

@Anushi1998, yea, you're right! I always forget about that. I'll try to fix it rebasing it. Thanks for the reminder.

@magsout

You can ping many people if needed

Okay! :)
I saw your comment on the issue and I'll change the variable names.
Thanks!

[Anushi1998]

@marimeireles I don't know if it would be better, but yes shorter you might consider trying git commit --amend

[marimeireles]

@Anushi1998 According to Github's doc "If the commit only exists in your local repository and has not been pushed to GitHub, you can amend the commit message with the git commit --amend command."
So I don't think it works when you already pushed.

[laghee]

@marimeireles You can still use amend if you've pushed, but then you have to force push 😱. You should always be careful about push --f since it's not a good habit to get into, but sometimes it's the simplest fix. Keep in mind that amend will also automatically include any other changes you have made locally. I use this constantly: https://blog.github.com/2015-06-08-how-to-undo-almost-anything-with-git/

(Also fun -- if you don't mind some inappropriate language -- is this one:grin: )

[marimeireles]

That changes everything! o: I didn't know it.
Thanks for the tip @laghee! I'm totally going to use it! :)

And thanks for sharing the blog posts too. Super useful.

[Anushi1998]

@marimeireles Your CircleCI is failing with same error as #2662 (comment).

The possible fix is to update your branch with master and rebase you work :)

[marimeireles]

Ahmm. You're right!
Thanks for that! You saved me a lot of debugging time :)

[marimeireles]

Sorry to ping you, I just found a bug at my implementation. :<
I'll fix it and ping you again when it's ready.

[marimeireles]

Hi @magsout, I think it's working now.
Please let me know if there's anything I can improve :)

[magsout]

@marimeireles I noted some improvements.

Thanks a lot for your help.

I ping @miketaylr for tests, because I'm not sure, again @miketaylr and also @karlcow for the logic of the behavior.

[marimeireles]

Thanks for the improvements, @magsout!
(Sorry for the commit mess, I'll fix it once all my changes are approved).

[karlcow]

The rest looks good in terms of what the UI.
Just needs to remove the index file into the webhooks directory.

[marimeireles]

@magsout, @karlcow do you think its good enough to merge?
Now I just have to organize my commits, right?

[karlcow]

@marimeireles on my side yes, you just need to rebase the commits so that it's less mistakes fixing and more logical code organization. Thanks a lot.

[magsout]

@marimeireles yes +1, good job and again thanks.

[marimeireles]

Thank you for all the help :)
I fixed the commits.

[karlcow]

once @magsout has approved on his side. I will merge.

[magsout]

@karlcow approved
@marimeireles 👋