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

      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
      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 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.
      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.

        People

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

          Dates

          • Created:
            Updated:
            Resolved:

            Development