Skip to content

Add proto3 syntax check#352

Merged
jdsika merged 1 commit intomasterfrom
add-proto3-syntax-check
Nov 5, 2019
Merged

Add proto3 syntax check#352
jdsika merged 1 commit intomasterfrom
add-proto3-syntax-check

Conversation

@vkresch
Copy link
Copy Markdown
Contributor

@vkresch vkresch commented Nov 5, 2019

Reference to a related issue in the repository

Related issue. Just for more test coverage in the CI pipeline which includes also the protobuf 3 syntax through the usage of the script convert-to-proto3.sh.

Add a description

What is this change?
This PR divides the travis ci into two stages one for testing and one for depolying. Inside the stage testing there will be executed two jobs in parallel. One job is for testing the build with protobuf 2 syntax the other job is for testing the build with protobuf 3. The deploy stage has the same code as the protobuf 2 test because currently you can't share files between jobs or stages see quote below from here:

It is important to note that jobs do not share storage, as each job runs in a fresh VM or container. If your jobs need to share files (e.g., using build artifacts from the “Test” stage for deployment in the subsequent “Deploy” stage), you need to use an external storage mechanism such as S3 and a remote scp server.

What does it fix?
Covers the testing for the protobuf 3 syntax if it is able to build.

How has it been tested?
In my fork here you can see the builds of each stage. See also the current travis build.

Mention a member

@jdsika pls review.

Check the checklist

  • My code and comments follow the style guidelines and contributors guidelines of this project.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests / travis ci pass locally with my changes.

@jdsika jdsika added this to the v3.1.3 milestone Nov 5, 2019
@jdsika jdsika added the Quality Quality improvements. label Nov 5, 2019
Comment thread .travis.yml
- cmake -D CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install ..
- cmake --build .
- cd ..
name: 'Test protobuf 3.0.0 syntax'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be positioned above "test: "?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me check on my fork if it works

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to merge in the next 30min if possible :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason it needs to be under scripts

@jdsika jdsika merged commit 030ec2d into master Nov 5, 2019
@vkresch vkresch deleted the add-proto3-syntax-check branch November 6, 2019 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Quality Quality improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants