You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Brown (Code Review)" <ge...@cloudera.org> on 2017/11/27 23:23:34 UTC

[Impala-ASF-CR] [DOCS] include version when building

Michael Brown has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8654


Change subject: [DOCS] include version when building
......................................................................

[DOCS] include version when building

This patch includes the canonical Impala version from
bin/save-version.sh in the doc builds' PDF and HTML output.

- Move impala_keydefs.ditamap to impala_keydefs.ditamap.in: this is now
  the checked in file.
- Include a marker in impala_keydefs.ditamap.in to replace the
  $IMPALA_VERSION when running "make".
- Use bin/save-version.sh to generate an updated version.info for the
  docs build. This is kept separate from bin/version.info since a full
  Impala build should not be required when just building the docs.
- Make the generated impala_keydefs.ditamap depend on both its source
  template and version.info; generate it using bash. Set SHELL so that
  we can use set -u and set -o pipefail (/bin/sh does not provide
  these).
- Update README on how to set IMPALA_HOME: this is needed so the
  absolute location of bin/save-version.sh can be known.

Testing:
"make" generated both PDF and HTML, and the version string was evident
and correct in both.

Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
---
M docs/.gitignore
M docs/Makefile
M docs/README.md
M docs/impala.ditamap
R docs/impala_keydefs.ditamap.in
5 files changed, 36 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/8654/1
-- 
To view, visit http://gerrit.cloudera.org:8080/8654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
Gerrit-Change-Number: 8654
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] [DOCS] include version when building

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8654 )

Change subject: [DOCS] include version when building
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8654/1/docs/Makefile
File docs/Makefile:

http://gerrit.cloudera.org:8080/#/c/8654/1/docs/Makefile@31
PS1, Line 31: 	set -eu -o pipefail ;\
            : 	export IMPALA_VERSION=$$(grep VERSION: version.info \
            :         | awk '{print $$2}' | sed -e 's/-SNAPSHOT//') ;\
            : 	sed -e "s/\$$IMPALA_VERSION/$$IMPALA_VERSION/" < $< > $@
> Maybe make this a separate script file that can be run and tested outside o
Sure. I'll fix it if we decide to move forward here.



-- 
To view, visit http://gerrit.cloudera.org:8080/8654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
Gerrit-Change-Number: 8654
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Nov 2017 17:53:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] include version when building

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has abandoned this change. ( http://gerrit.cloudera.org:8080/8654 )

Change subject: [DOCS] include version when building
......................................................................


Abandoned

Not working on this, not enough apparent interest among stake holders.
-- 
To view, visit http://gerrit.cloudera.org:8080/8654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
Gerrit-Change-Number: 8654
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] [DOCS] include version when building

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8654 )

Change subject: [DOCS] include version when building
......................................................................


Patch Set 1:

> In general, I would like to keep a checked-in version of
> impala_keydefs.ditamap so that there was always a way to do a
> single 'dita ...' command to run a build for basic proofreading,
> even if the production-ready technique went through extra steps.

If this is the case, I'll have to think about this, then. You typically want to keep files under source control separate from artifacts generated by a build process. When you modify files under source control in-place, the modifications can leak into commits. In this case, you'd have to make sure the "version marker" never gets modified by a human who ran "make" locally.


-- 
To view, visit http://gerrit.cloudera.org:8080/8654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
Gerrit-Change-Number: 8654
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Nov 2017 16:24:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] include version when building

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has posted comments on this change. ( http://gerrit.cloudera.org:8080/8654 )

Change subject: [DOCS] include version when building
......................................................................


Patch Set 1:

I'm not going to have time to think this through in detail today.

I have on my backburner to make some general enhancements to the Makefile, e.g. some more phony targets to build, XML validate, etc. combinations of things.

In general, I would like to keep a checked-in version of impala_keydefs.ditamap so that there was always a way to do a single 'dita ...' command to run a build for basic proofreading, even if the production-ready technique went through extra steps.

I think the <keydef> tag I added in https://gerrit.cloudera.org/#/c/8648/ could serve as the placeholder if desired. We could keep a dummy value in the source file and always substitute during the build. Although... there's where I would need more time to look through the suggestion and think through how it matches with the current scheme.


-- 
To view, visit http://gerrit.cloudera.org:8080/8654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
Gerrit-Change-Number: 8654
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Nov 2017 23:58:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] include version when building

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8654 )

Change subject: [DOCS] include version when building
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8654/1/docs/Makefile
File docs/Makefile:

http://gerrit.cloudera.org:8080/#/c/8654/1/docs/Makefile@31
PS1, Line 31: 	set -eu -o pipefail ;\
            : 	export IMPALA_VERSION=$$(grep VERSION: version.info \
            :         | awk '{print $$2}' | sed -e 's/-SNAPSHOT//') ;\
            : 	sed -e "s/\$$IMPALA_VERSION/$$IMPALA_VERSION/" < $< > $@
> I get that this is a little gross. Eager to hear any advice on cleanup.
Maybe make this a separate script file that can be run and tested outside of this Makefile?



-- 
To view, visit http://gerrit.cloudera.org:8080/8654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2036993636c8f73c07790bd4bdf4da0b721549a
Gerrit-Change-Number: 8654
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Nov 2017 04:54:59 +0000
Gerrit-HasComments: Yes