Moodle
  1. Moodle
  2. MDL-30283

general.css of anomaly theme has git merge tags: <<<<<<< HEAD and etc.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.5, 2.1.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide

      It might be a good idea to test the custom menu as the code I had added originally was from Moodle 2.2 which caused the conflict as it should not have been added. Mea Culpa! I have now removed that code, and made good the original YUI-reset.

      Show
      It might be a good idea to test the custom menu as the code I had added originally was from Moodle 2.2 which caused the conflict as it should not have been added. Mea Culpa! I have now removed that code, and made good the original YUI-reset.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Rank:
      32624

      Issue Links

        Activity

        Hide
        Michael de Raadt added a comment -

        This appears to be a recent regression in 2.1 only. I checked 2.2 and it doesn't seem to have the same problem. I'll try to track down the issue related to the regression.

        Show
        Michael de Raadt added a comment - This appears to be a recent regression in 2.1 only. I checked 2.2 and it doesn't seem to have the same problem. I'll try to track down the issue related to the regression.
        Show
        Michael de Raadt added a comment - This affects the 2.0 branch as well. Here are the relevant commits: 2.0: http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=c9b3258ccce481b3e8be32434e822f0d1f166169 2.1: http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=7946da5f3c6b6564e740adcceaae112fbe07ba66
        Hide
        Mary Evans added a comment -

        How did this happen?

        Show
        Mary Evans added a comment - How did this happen?
        Hide
        Mary Evans added a comment - - edited

        This is fast turning into a nightmare!
        What the hell is wrong with this theme!
        It's picking up all sorts of rubbish from who knows where!
        I am going to look at this again tomorrow.

        Show
        Mary Evans added a comment - - edited This is fast turning into a nightmare! What the hell is wrong with this theme! It's picking up all sorts of rubbish from who knows where! I am going to look at this again tomorrow.
        Hide
        Mary Evans added a comment - - edited

        I have just come across some CSS mark-up in general.css which is not in the original CSS files I changed.

        #page-report-outline-user .section {
            border:i 1px solid #DDD;
            margin: 0 5% 1.5em 5%;
        }
        
        #page-report-outline-user .section h2,
        #page-report-outline-user .section .content {
            margin: 5px 1em;
        }
        

        Also there is an ERROR in this code

            border:i 1px solid #DDD;
        

        border: i ?????

        So whoever did this probably screwed up the file.

        Thanks a bunch...
        Just what I can do without

        Mary

        Show
        Mary Evans added a comment - - edited I have just come across some CSS mark-up in general.css which is not in the original CSS files I changed. #page-report-outline-user .section { border:i 1px solid #DDD; margin: 0 5% 1.5em 5%; } #page-report-outline-user .section h2, #page-report-outline-user .section .content { margin: 5px 1em; } Also there is an ERROR in this code border:i 1px solid #DDD; border: i ????? So whoever did this probably screwed up the file. Thanks a bunch... Just what I can do without Mary
        Hide
        Mary Evans added a comment - - edited

        This is the file I pushed to my GIT Repository in the commit:

        wip-MDL-22351 convert Anomaly CSS to readable format...

        https://github.com/lazydaisy/moodle/blob/0a52611c5a253a241009634e7ee32fc01ee9a0f4/theme/anomaly/style/general.css

        So what I would like to know where did all that other CSS mark-up come from?

        Cheers
        Mary

        Show
        Mary Evans added a comment - - edited This is the file I pushed to my GIT Repository in the commit: wip- MDL-22351 convert Anomaly CSS to readable format... https://github.com/lazydaisy/moodle/blob/0a52611c5a253a241009634e7ee32fc01ee9a0f4/theme/anomaly/style/general.css So what I would like to know where did all that other CSS mark-up come from? Cheers Mary
        Hide
        Mary Evans added a comment -

        Also...which bits do I actually remove...

        If the YUICSS has been added there then that's fine I'll clean that up too, but as it is it looks like a complete dogs dinner!

        Show
        Mary Evans added a comment - Also...which bits do I actually remove... If the YUICSS has been added there then that's fine I'll clean that up too, but as it is it looks like a complete dogs dinner!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Bummer!

        When I integrated this I found various conflicts caused because, the same week, other changes were performed to the same lines in anomaly theme by another issue.

        Usually we reject those conflicts. But knowing that your change was just a "whitespace clean" I did perform the merge, respecting your whitespace format, but applying the changes to selectors performed by the other issue (was a big modification affecting to "report" related styles).

        And I triple checked that the final result was satisfactory before pushing that to the git repo. In fact both my merge tool and git should have warned me about remaining <<<< lines there, grrr.

        Going to confirm that (and fix it) ASAP, still I don't understand how that escaped all the checks I've here before sending stuff. Apologizes!

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Bummer! When I integrated this I found various conflicts caused because, the same week, other changes were performed to the same lines in anomaly theme by another issue. Usually we reject those conflicts. But knowing that your change was just a "whitespace clean" I did perform the merge, respecting your whitespace format, but applying the changes to selectors performed by the other issue (was a big modification affecting to "report" related styles). And I triple checked that the final result was satisfactory before pushing that to the git repo. In fact both my merge tool and git should have warned me about remaining <<<< lines there, grrr. Going to confirm that (and fix it) ASAP, still I don't understand how that escaped all the checks I've here before sending stuff. Apologizes! Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment - - edited

        Super-bummer!

        Just was looking for the problems... and they are only under 2.0 and 2.1 ? That throws away my theory about the conflicts I got in master with the changes related to reports.

        Aha, yes, the >>>> were not introduced by my merge done improperly, but your original commits (pointed by Michael above) already had them. That's the reason the problem was not caught here because they were "existing" commits that are not checked.

        So I guess that:

        1) you did the original cleanup in master.
        2) you cherry-picked/merged those changes back to 2.0 and 2.1
        3) The 2) operation, should have shown you a BIG-RED warning about the conflicts, but somehow it didn't/you forgot it and instructed git to continue (aka commit).

        Anyway, there are 2 solutions for this:

        A) The quickest, I just can introduce one extra commit sending back the status of the theme to its original code (before the offending commit arrived). I can do it now and send it to all repos (it's called an emergency build)
        B) The best, you are able to fix it, solve all the conflicts and send this to integration and it will be available in tomorrow's normal weekly builds.

        If I can help with any of A), B) plz tell me. Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - - edited Super-bummer! Just was looking for the problems... and they are only under 2.0 and 2.1 ? That throws away my theory about the conflicts I got in master with the changes related to reports. Aha, yes, the >>>> were not introduced by my merge done improperly, but your original commits (pointed by Michael above) already had them. That's the reason the problem was not caught here because they were "existing" commits that are not checked. So I guess that: 1) you did the original cleanup in master. 2) you cherry-picked/merged those changes back to 2.0 and 2.1 3) The 2) operation, should have shown you a BIG-RED warning about the conflicts, but somehow it didn't/you forgot it and instructed git to continue (aka commit). Anyway, there are 2 solutions for this: A) The quickest, I just can introduce one extra commit sending back the status of the theme to its original code (before the offending commit arrived). I can do it now and send it to all repos (it's called an emergency build) B) The best, you are able to fix it, solve all the conflicts and send this to integration and it will be available in tomorrow's normal weekly builds. If I can help with any of A), B) plz tell me. Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        BTW, I'm adding a new test here, so anything landing to git repos and having any <<|>> merge annotations will be detected and reported quickly. I hope that will help to detect any problem like this in minutes, so a quick revert/reject can be done.

        Show
        Eloy Lafuente (stronk7) added a comment - BTW, I'm adding a new test here, so anything landing to git repos and having any <<|>> merge annotations will be detected and reported quickly. I hope that will help to detect any problem like this in minutes, so a quick revert/reject can be done.
        Hide
        Mary Evans added a comment -

        Hi
        That was an awful nights sleep I got!

        OK...I'll go for option "B" as that seems more logical to me.

        Now you mention it I did have some problems cherry picking and could not understand why that file caused so much of a problem, Now I know why. However, I don't fully understand how GIT works to have been able to deduce what was going on.

        Well, like I keep saying, "you learn by your mistakes" or as some great person, I forget the name of, once said "If you've never made a mistake, you've never made anything".

        Thanks and ciao.
        Mary

        Show
        Mary Evans added a comment - Hi That was an awful nights sleep I got! OK...I'll go for option "B" as that seems more logical to me. Now you mention it I did have some problems cherry picking and could not understand why that file caused so much of a problem, Now I know why. However, I don't fully understand how GIT works to have been able to deduce what was going on. Well, like I keep saying, "you learn by your mistakes" or as some great person, I forget the name of, once said "If you've never made a mistake, you've never made anything". Thanks and ciao. Mary
        Hide
        Mary Evans added a comment -

        Hi Eloy,
        I have just pushed one branch only MDL-30283-M20...if this is OK can you cherry-pick this to MOODLE_21_STABLE...???

        Thanks
        Mary

        Show
        Mary Evans added a comment - Hi Eloy, I have just pushed one branch only MDL-30283 -M20...if this is OK can you cherry-pick this to MOODLE_21_STABLE...??? Thanks Mary
        Hide
        Eloy Lafuente (stronk7) added a comment -

        well, if there are no differences between 20_STABLE and 21_STABLE in areas affected by your changes, yes, then it can be cherry-picked. I'll try and let you know in 1h or so.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - well, if there are no differences between 20_STABLE and 21_STABLE in areas affected by your changes, yes, then it can be cherry-picked. I'll try and let you know in 1h or so. Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I've cherry-picked the 20_STABLE branch (1 commit) into 21_STABLE and it was applied without any error, so I guess we can send this to integration (note I've not verified that the CSS are the correct ones nor anything like that). But, for sure, there are no pending '<<<<' conflicts at all.

        Let's the testers check the custom menus (I've switched here to anomaly my 2.1 test server, I use the custom menus for some quick links in all my test sites and they seem to be working ok.

        So, is it ok if I integrate this? Thanks for the quick fix, btw!

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - I've cherry-picked the 20_STABLE branch (1 commit) into 21_STABLE and it was applied without any error, so I guess we can send this to integration (note I've not verified that the CSS are the correct ones nor anything like that). But, for sure, there are no pending '<<<<' conflicts at all. Let's the testers check the custom menus (I've switched here to anomaly my 2.1 test server, I use the custom menus for some quick links in all my test sites and they seem to be working ok. So, is it ok if I integrate this? Thanks for the quick fix, btw! Ciao
        Hide
        Mary Evans added a comment -

        Hi,

        Yes please do get this integrated ASAP.
        I have just checked the CSS over again, and it's OK.

        You were right it was because I had made all my changes in Master, not thinking that it was any different than Moodle 2.1 & Moodle 2.00, how wrong could I be.

        Thanks too for your help in getting this put right.

        Cheers
        Mary

        Show
        Mary Evans added a comment - Hi, Yes please do get this integrated ASAP. I have just checked the CSS over again, and it's OK. You were right it was because I had made all my changes in Master, not thinking that it was any different than Moodle 2.1 & Moodle 2.00, how wrong could I be. Thanks too for your help in getting this put right. Cheers Mary
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
        Hide
        Michael de Raadt added a comment -

        Test result: fixed. Sorry if this caused you grief.

        Show
        Michael de Raadt added a comment - Test result: fixed. Sorry if this caused you grief.
        Hide
        Mary Evans added a comment -

        @ Michael,
        It did cause me to do a "Teresa May" on this...blaming everyone and everything except myself
        It was not until Eloy pointed out what I had done that I realised what a mess I had got myself into. Let's say, 'it was a wake-up call!'
        I have learnt a lot from that mistake, and I certainly hope I don't encounter it again...at least not in a long while...if ever!

        Anomaly is sure living up to it's name though! LOL

        Ciao
        Mary

        Show
        Mary Evans added a comment - @ Michael, It did cause me to do a "Teresa May" on this...blaming everyone and everything except myself It was not until Eloy pointed out what I had done that I realised what a mess I had got myself into. Let's say, 'it was a wake-up call!' I have learnt a lot from that mistake, and I certainly hope I don't encounter it again...at least not in a long while...if ever! Anomaly is sure living up to it's name though! LOL Ciao Mary
        Hide
        Eloy Lafuente (stronk7) added a comment -

        U P S T R E A M I Z E D !

        Thanks! Closing...

        Show
        Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Thanks! Closing...

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: