Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Internal] Parser: Adds Antlr Dependancy #1691

Merged
merged 15 commits into from Sep 23, 2020
Merged

Conversation

bchong95
Copy link
Contributor

@bchong95 bchong95 commented Jul 8, 2020

Internal Parser: Merge Antlr Dependancy

Description

Copies over the Antlr runtime, but makes all the top level types internal and adds a auto generated tag, so that stylecop ignores these files. We can add other OSS dependencies here to avoid the nuget package dependency.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please follow the required format: "[Internal] Category: (Add|Fix|Refactor) Description"

Examples:
Diagnostics: Add GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fix null reference when using default(PartitionKey)
[v4] Client Encryption: Refactor code to external project
[Internal] Query: Add code generator for CosmosNumbers for easy additions in the future.

@bchong95 bchong95 changed the title Internal Parser: Merge Antlr Dependancy [Internal] Parser: Add Antlr Dependancy Jul 8, 2020
@github-actions github-actions bot dismissed their stale review July 8, 2020 21:21

All good!

sboshra
sboshra previously approved these changes Jul 10, 2020
Copy link
Contributor

@sboshra sboshra left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@j82w j82w left a comment

Choose a reason for hiding this comment

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

Blocking until license requirements are clarified.

// This file isn't generated, but this comment is necessary to exclude it from StyleCop analysis.
// <auto-generated/>

/* Copyright (c) 2012-2017 The ANTLR Project. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

@kirankumarkolli is there clarification on requirements for the license?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

He literally said in the last meeting that he would work with the legal team about the licensing and that there is no need to block on it. I feel like we attend different meetings at this point. From now on I am requesting that all meetings are recorded for transparency.

Copy link
Contributor

@j82w j82w Jul 13, 2020

Choose a reason for hiding this comment

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

If this merged before legal is done then master is in a broken state and we can't do a release. Having master in a state where it can't be shipped is unacceptable, and worse is if someone does do a release it becomes a legal issue. The meeting recording is here. I might have missed it but the only agreement I found and remember was @kirankumarkolli agreeing to follow up with legal. I did not find anything saying this would be allowed to be merged without legal being done first.

Copy link
Member

Choose a reason for hiding this comment

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

Let me clarify here.

  1. Work progress (working, PR, reviews etc...) don't need to get blocked on legal clearance (very high probability of clearance).
  2. PR merge do need clearance, so its a blocker.
  3. I am supposed to start the legal process last week and slipped it. Will do submit today.

@j82w j82w changed the title [Internal] Parser: Add Antlr Dependancy [Internal] Parser: Adds Antlr Dependancy Jul 14, 2020
@bchong95 bchong95 self-assigned this Jul 16, 2020
@sboshra sboshra merged commit 5f7dd88 into master Sep 23, 2020
@sboshra sboshra deleted the users/brchon/AntlrCopyPaste branch September 23, 2020 16:33
@ghost
Copy link

ghost commented Dec 15, 2021

Closing due to in-activity, pease feel free to re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants