Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.5.0
    • Component/s: general
    • Labels:
      None
    1. BIGTOP-695.patch
      3 kB
      Anatoli Fomenko
    2. BIGTOP-695.patch
      3 kB
      Anatoli Fomenko

      Activity

      Anatoli Fomenko created issue -
      Anatoli Fomenko made changes -
      Field Original Value New Value
      Attachment BIGTOP-695.patch [ 12543737 ]
      Hide
      Anatoli Fomenko added a comment -

      Patch submitted. Please review.
      I used install_pig.sh as an inspiration.

      Show
      Anatoli Fomenko added a comment - Patch submitted. Please review. I used install_pig.sh as an inspiration.
      Anatoli Fomenko made changes -
      Status Open [ 1 ] Patch Available [ 10002 ]
      Hide
      Bruno Mahé added a comment -

      Thanks a lot Anatoli for this patch!
      It goes along what I was thinking when this ticket was opened (sorry for not being more descriptive), but there are a few things that could be fixed:

      •         --flume-dir)
         -        FLUME_DIR=$2 ; shift 2
         +        LIB_DIR=$2 ; shift 2
         

        If you look at Apache Flume's rules or spec file, --flume-dir is never used. So we could remove it or better, rename it to --lib-dir for consistency

      • I would rather keep the PREFIX out of the variables. This makes it easier to use the install script (compare the spec files of hadoop where all parameters look like _--doc-dir=$RPM_BUILD_ROOT% {doc_hadoop}

        _ [in addition of already passing the prefix as another parameter] versus hbase where they look like _--doc-dir=%

        {doc_hbase}

        _).
        In order to avoid a repetition of the pattern <PREFIX>/<DIR> from within the script, feel free to create another set of variables or any other method of your choice. But the lib/man/bin/etc. directories passed to the scripts should not be already prefixed.

      Show
      Bruno Mahé added a comment - Thanks a lot Anatoli for this patch! It goes along what I was thinking when this ticket was opened (sorry for not being more descriptive), but there are a few things that could be fixed: --flume-dir) - FLUME_DIR=$2 ; shift 2 + LIB_DIR=$2 ; shift 2 If you look at Apache Flume's rules or spec file, --flume-dir is never used. So we could remove it or better, rename it to --lib-dir for consistency I would rather keep the PREFIX out of the variables. This makes it easier to use the install script (compare the spec files of hadoop where all parameters look like _--doc-dir=$RPM_BUILD_ROOT% {doc_hadoop} _ [in addition of already passing the prefix as another parameter] versus hbase where they look like _--doc-dir=% {doc_hbase} _). In order to avoid a repetition of the pattern <PREFIX>/<DIR> from within the script, feel free to create another set of variables or any other method of your choice. But the lib/man/bin/etc. directories passed to the scripts should not be already prefixed.
      Anatoli Fomenko made changes -
      Attachment BIGTOP-695.patch [ 12544381 ]
      Hide
      Anatoli Fomenko added a comment -

      Thanks Bruno for your feedback.

      Please find a new iteration of my patch. In a nutshell, I kept PREFIX out of the variables; replaced flume-dir with lib-dir and FLUME_DIR with LIB_DIR; removed unused variables CONF_DIR and INSTALLED_LIB_DIR.

      Please review.
      Thank you.

      Show
      Anatoli Fomenko added a comment - Thanks Bruno for your feedback. Please find a new iteration of my patch. In a nutshell, I kept PREFIX out of the variables; replaced flume-dir with lib-dir and FLUME_DIR with LIB_DIR; removed unused variables CONF_DIR and INSTALLED_LIB_DIR. Please review. Thank you.
      Hide
      Bruno Mahé added a comment -

      Thanks a lot for updating your patch!

      I reviewed it and LGTM.
      +1

      Show
      Bruno Mahé added a comment - Thanks a lot for updating your patch! I reviewed it and LGTM. +1
      Bruno Mahé made changes -
      Status Patch Available [ 10002 ] Resolved [ 5 ]
      Resolution Fixed [ 1 ]
      Bruno Mahé made changes -
      Status Resolved [ 5 ] Closed [ 6 ]
      Transition Time In Source Status Execution Times Last Executer Last Execution Date
      Open Open Patch Available Patch Available
      1d 15h 24m 1 Anatoli Fomenko 04/Sep/12 21:23
      Patch Available Patch Available Resolved Resolved
      7d 9h 59m 1 Bruno Mahé 12/Sep/12 07:23
      Resolved Resolved Closed Closed
      6m 48s 1 Bruno Mahé 12/Sep/12 07:29

        People

        • Assignee:
          Anatoli Fomenko
          Reporter:
          Anatoli Fomenko
        • Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development