You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/04/30 08:14:48 UTC

[GitHub] [flink-docker] zentol opened a new pull request #18: [FLINK-17439] Introduce development branches

zentol opened a new pull request #18:
URL: https://github.com/apache/flink-docker/pull/18


   - remove generates/templates; these now reside in the development branches
   - remove testing files; these now reside in the development branches
   - update README.md to reflect new development/release workflow


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] zentol commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r420969292



##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       I'm undecided at this point. the dev-1.x branches work against releases to keep existing behavior the same. dev-master must go against snapshots, because there's nothing else to test against.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] azagrebin commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r420860393



##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       It would be nice to add some details about which flink dist is tested by CI on dev branches.
   
   It looks that the `dev-1.X` branches run CI always against the last release and not its snapshots, like in `dev-master`, do we want this? This makes sense to check changes for generating other Dockerfile for the current patch release but it does not prepare for the next patch release using major release snapshots




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] zentol commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r421326344



##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       I suppose what currently will happen is that the dev-1.x branches work against release Flink versionsd (so, docker-1.x-snapshot tests against Flink 1.x), while the Flink release-1.x branches work again released docker images (so, flink-1.x-snapshot tests against docker 1.x).
   
   This is fine I think; the above should give us enough guarantees that we don't need to explicitly test for snapshot <-> snapshot.

##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       I suppose what currently will happen is that the dev-1.x branches work against release Flink versions (so, docker-1.x-snapshot tests against Flink 1.x), while the Flink release-1.x branches work again released docker images (so, flink-1.x-snapshot tests against docker 1.x).
   
   This is fine I think; the above should give us enough guarantees that we don't need to explicitly test for snapshot <-> snapshot.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] zentol commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r422836564



##########
File path: README.md
##########
@@ -34,6 +34,13 @@ Development happens on the various `dev-X` branches.
 Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
 Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.
 
+### CI
+
+The `dev-master` branch is tested against nightly Flink snapshots for the next major Flink version. This allows us to
+develop features in tandem with Flink.
+
+The `dev-1.x` branches are tested against the latest corresponding minor Flink release, to ensure any changes we make

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] azagrebin commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r420860229



##########
File path: README.md
##########
@@ -39,24 +50,30 @@ There are additional steps required when a new Flink minor version (x.y.0) is re
 
 ### Release workflow
 
-When a new release of Flink is available, the Dockerfiles in this repo should be updated and a new
+When a new release of Flink is available, the Dockerfiles in the `master` branch should be updated and a new
 manifest sent to the Docker Library [`official-images`](
 https://github.com/docker-library/official-images) repo.
 
-Updating the Dockerfiles involves 3 steps:
-
-1. Update `add-version.sh` with the GPG key ID of the key used to sign the new release
-    * Commit this change with message `Add GPG key for x.y.z release` <sup>\[[example](
-      https://github.com/apache/flink-docker/commit/94845f46c0f0f2de80d4a5ce309db49aff4655d0)]</sup>
-2. Remove any existing Dockerfiles from the same minor version
-    * e.g. `rm -r 1.2`, if the new Flink version is `1.2.1`
-3. Run `add-version.sh` with the appropriate arguments (`-r flink-minor-version -f
-   flink-full-version`)
-    * e.g. `./add-version.sh -r 1.2 -f 1.2.1`
-    * Commit these changes with message `Update Dockerfiles for x.y.z release` <sup>\[[example](
+The Dockerfiles are generated on the respective `dev-<version>` branches, and copied over to the `master` branch for
+publishing.
+
+Updating the Dockerfiles involves the following steps:
+
+1. Generate the Dockerfiles
+    * Checkout the `dev-x.y` branch of the respective release, e.g., dev-1.9
+    * Update `add-version.sh` with the GPG key ID of the key used to sign the new release

Review comment:
       do we need to keep GPG keys in repo mostly for CI?

##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       It would be nice to add some details about which flink dist is tested by CI on dev branches.
   
   It looks that the `dev-1.X` branches run CI always against the last release and not its snapshots, like in `dev-master`, do we want this? This makes sense to check changes for generating other Dockerfile for the current minor release but it does not prepare for the next minor release using major release snapshots

##########
File path: README.md
##########
@@ -79,14 +96,17 @@ available shortly thereafter.
 
 ### Release checklist
 
+Checklist for the `dev` branch:
 - [ ] The GPG key ID of the key used to sign the release has been added to `add-version.sh` and
       committed with the message `Add GPG key for x.y.z release`
+- [ ] `./add-version.sh -r x.y -f x.y.z` has been run on the respective dev branch
+
+Checklist for the `master` branch:
+- [ ] The updated Dockerfiles have been committed with the message `Update Dockerfiles for x.y.z release`

Review comment:
       I think this step `updated Dockerfiles` should be either after or merged into the current 3rd step:
   `_(new patch releases only)_ any existing generated files for the same minor version have been removed`
   as before the step `./add-version.sh..` was.
   As I understand new `x.y.z` overwrites the previous `x.y.(z-1)`.
   Otherwise, it is confusing, imo




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] azagrebin commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r421389278



##########
File path: README.md
##########
@@ -34,6 +34,13 @@ Development happens on the various `dev-X` branches.
 Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
 Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.
 
+### CI
+
+The `dev-master` branch is tested against nightly Flink snapshots for the next major Flink version. This allows us to
+develop features in tandem with Flink.
+
+The `dev-1.x` branches are tested against the latest corresponding minor Flink release, to ensure any changes we make

Review comment:
       ```suggestion
   The `dev-1.x` branches are tested against the latest corresponding patch Flink release, to ensure any changes we make
   ```
   I would either push a hotfix changing overall in this file:
   minor -> major
   patch -> minor
   or follow the terms we have here and create an issue for the above change.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] azagrebin commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r420860393



##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       It would be nice to add some details about which flink dist is tested by CI on dev branches.
   
   It looks that the `dev-1.X` branches run CI always against the last release and not its snapshots, like in `dev-master`, do we want this?
   This makes sense to check changes for generating other Dockerfile for the current patch release but it does not prepare for the next patch release using major release snapshots. Do you think it is not so important difference atm?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] zentol commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r422834190



##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       yes; the e2e tests will likely checkout a specific flink-docker commit and build the Dockerfiles against a local distribution.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] zentol commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r421327322



##########
File path: README.md
##########
@@ -39,24 +50,30 @@ There are additional steps required when a new Flink minor version (x.y.0) is re
 
 ### Release workflow
 
-When a new release of Flink is available, the Dockerfiles in this repo should be updated and a new
+When a new release of Flink is available, the Dockerfiles in the `master` branch should be updated and a new
 manifest sent to the Docker Library [`official-images`](
 https://github.com/docker-library/official-images) repo.
 
-Updating the Dockerfiles involves 3 steps:
-
-1. Update `add-version.sh` with the GPG key ID of the key used to sign the new release
-    * Commit this change with message `Add GPG key for x.y.z release` <sup>\[[example](
-      https://github.com/apache/flink-docker/commit/94845f46c0f0f2de80d4a5ce309db49aff4655d0)]</sup>
-2. Remove any existing Dockerfiles from the same minor version
-    * e.g. `rm -r 1.2`, if the new Flink version is `1.2.1`
-3. Run `add-version.sh` with the appropriate arguments (`-r flink-minor-version -f
-   flink-full-version`)
-    * e.g. `./add-version.sh -r 1.2 -f 1.2.1`
-    * Commit these changes with message `Update Dockerfiles for x.y.z release` <sup>\[[example](
+The Dockerfiles are generated on the respective `dev-<version>` branches, and copied over to the `master` branch for
+publishing.
+
+Updating the Dockerfiles involves the following steps:
+
+1. Generate the Dockerfiles
+    * Checkout the `dev-x.y` branch of the respective release, e.g., dev-1.9
+    * Update `add-version.sh` with the GPG key ID of the key used to sign the new release

Review comment:
       But yes, currently we need it for CI for the dev-1.x, but this would be trivial to change.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] azagrebin commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r421363940



##########
File path: README.md
##########
@@ -24,6 +24,17 @@ Flink Docker image lifecycle
   https://github.com/docker-library/official-images/blob/master/library/flink).
 
 
+Development workflow
+----------------------------
+
+The `master` branch of this repository serves as a pure publishing area for releases.
+
+Development happens on the various `dev-X` branches.
+
+Pull requests for a specific version should be opened against the respective `dev-<version>` branch.
+Pull requests for all versions, or for the next major Flink release, should be opened against the `dev-master` branch.

Review comment:
       How does `flink-1.x-snapshot tests against docker 1.x` work?
   Is it about the refactoring of flink docker e2e tests to use the cloned `flink-docker` `dev-1.x` branch?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-docker] zentol commented on a change in pull request #18: [FLINK-17439] Introduce development branches

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #18:
URL: https://github.com/apache/flink-docker/pull/18#discussion_r420967680



##########
File path: README.md
##########
@@ -39,24 +50,30 @@ There are additional steps required when a new Flink minor version (x.y.0) is re
 
 ### Release workflow
 
-When a new release of Flink is available, the Dockerfiles in this repo should be updated and a new
+When a new release of Flink is available, the Dockerfiles in the `master` branch should be updated and a new
 manifest sent to the Docker Library [`official-images`](
 https://github.com/docker-library/official-images) repo.
 
-Updating the Dockerfiles involves 3 steps:
-
-1. Update `add-version.sh` with the GPG key ID of the key used to sign the new release
-    * Commit this change with message `Add GPG key for x.y.z release` <sup>\[[example](
-      https://github.com/apache/flink-docker/commit/94845f46c0f0f2de80d4a5ce309db49aff4655d0)]</sup>
-2. Remove any existing Dockerfiles from the same minor version
-    * e.g. `rm -r 1.2`, if the new Flink version is `1.2.1`
-3. Run `add-version.sh` with the appropriate arguments (`-r flink-minor-version -f
-   flink-full-version`)
-    * e.g. `./add-version.sh -r 1.2 -f 1.2.1`
-    * Commit these changes with message `Update Dockerfiles for x.y.z release` <sup>\[[example](
+The Dockerfiles are generated on the respective `dev-<version>` branches, and copied over to the `master` branch for
+publishing.
+
+Updating the Dockerfiles involves the following steps:
+
+1. Generate the Dockerfiles
+    * Checkout the `dev-x.y` branch of the respective release, e.g., dev-1.9
+    * Update `add-version.sh` with the GPG key ID of the key used to sign the new release

Review comment:
       maybe. technically we only need the GPG key at release time, but I'm not sure yet whether we will not have a cron job running against the released images to check for breaking changes or smth.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org