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

Prepare nbs to ydb#1784 #508

Merged

Conversation

Enjection
Copy link
Member

We are going to disable PROTOC_TRANSITIVE_HEADERS in ydb/core/protos in ydb-platform/ydb#1784 to make this transition smooth for you I added additional include which brings back old behavior.
In case you want to speed up your builds as well you can remove this include and replace it with includes really needed.

@Enjection Enjection force-pushed the prepare-to-ydb-nontransitive-headers branch from d3b2623 to a7d5f1a Compare February 20, 2024 19:58
@Enjection
Copy link
Member Author

It's temporary on hold. Seem's that ymake don't understand conditional includes, so I have to think about different approach

Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit a7d5f1a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4743 4670 0 1 0 72

@Enjection Enjection force-pushed the prepare-to-ydb-nontransitive-headers branch from a7d5f1a to 547ca2f Compare February 20, 2024 21:01
@Enjection
Copy link
Member Author

Enjection commented Feb 20, 2024

Okay, I found workaround for this -- we will just ignore it with Y_IGNORE. It won't break anything as long as we include main header in same file

Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 547ca2f.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4743 4666 0 2 1 74

@Enjection
Copy link
Member Author

@qkrorlqr Could we discuss this change? It is blocking us

@qkrorlqr
Copy link
Collaborator

@qkrorlqr Could we discuss this change? It is blocking us

I don't see any results indicating that this activity improves something (build time?). Here ydb-platform/ydb#1784 I see lots of failed tests and builds in most of those PR-checks - I don't understand, how can we draw conclusions from such unstable measurements.

@Enjection Enjection force-pushed the prepare-to-ydb-nontransitive-headers branch from 547ca2f to 9290f36 Compare February 24, 2024 13:02
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 9290f36.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4743 4670 0 1 0 72

@Enjection Enjection force-pushed the prepare-to-ydb-nontransitive-headers branch from 1ec106c to ab37dc1 Compare February 26, 2024 11:48
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit ab37dc1.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
4750 4678 0 0 0 72

@qkrorlqr qkrorlqr merged commit a17f843 into ydb-platform:main Feb 26, 2024
7 checks passed
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.

3 participants