Skip to content

Conversation

ZolotukhinM
Copy link

@ZolotukhinM ZolotukhinM commented Sep 25, 2020

Stack from ghstack:

Differential Revision: D23935520

Copy link
Contributor

@Krovatkin Krovatkin 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

@wconstab wconstab left a comment

Choose a reason for hiding this comment

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

i'm fine with this, especially in the spirit of moving fast for PE work, but i also wonder if this approach (of running multiple configs on the same commit) makes sense long term, vs having the approach that each commit we measure in CI has a particular setting?

@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

❗ No coverage uploaded for pull request base (gh/ZolotukhinM/349/base@59014b4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                    Coverage Diff                     @@
##             gh/ZolotukhinM/349/base   #45348   +/-   ##
==========================================================
  Coverage                           ?   68.06%           
==========================================================
  Files                              ?      393           
  Lines                              ?    50987           
  Branches                           ?        0           
==========================================================
  Hits                               ?    34706           
  Misses                             ?    16281           
  Partials                           ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59014b4...68c3a44. Read the comment docs.

@ZolotukhinM
Copy link
Author

i'm fine with this, especially in the spirit of moving fast for PE work, but i also wonder if this approach (of running multiple configs on the same commit) makes sense long term, vs having the approach that each commit we measure in CI has a particular setting?

I have a similar concern but slightly swaying towards checking potentially redundant tests. Imho it's better than accidentally not testing what's important. In the long run we hopefully would remove the LE/OF configuration, and keep only the default, which would be PE/TE.

@facebook-github-bot
Copy link
Contributor

@ZolotukhinM merged this pull request in a07d829.

@facebook-github-bot facebook-github-bot deleted the gh/ZolotukhinM/349/head branch September 30, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants