Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30283

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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:

      Gliffy Diagrams

        Issue Links

          Activity

          Hide
          salvetore 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
          salvetore 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
          salvetore 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
          lazydaisy Mary Evans added a comment -

          How did this happen?

          Show
          lazydaisy Mary Evans added a comment - How did this happen?
          Hide
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          stronk7 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
          stronk7 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
          stronk7 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
          stronk7 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
          stronk7 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
          stronk7 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          stronk7 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
          stronk7 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
          stronk7 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
          stronk7 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
          lazydaisy 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
          lazydaisy 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
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

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

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

          Show
          salvetore Michael de Raadt added a comment - Test result: fixed. Sorry if this caused you grief.
          Hide
          lazydaisy 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
          lazydaisy 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
          stronk7 Eloy Lafuente (stronk7) added a comment -

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

          Thanks! Closing...

          Show
          stronk7 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:
                Fix Release Date:
                28/Nov/11