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

Make showfunc=True by default #1029

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stepancheg
Copy link
Contributor

@stepancheg stepancheg commented Feb 14, 2025

showfunc option is convenient, and sl does not need to have backwards compatibility, so turn the option on.

Note this is what git does by default.

sl diff
diff --git a/examples/ListPeople.java b/examples/ListPeople.java
--- a/examples/ListPeople.java
+++ b/examples/ListPeople.java
@@ -14,6 +14,7 @@ class ListPeople {
       System.out.println("  Name: " + person.getName());
       if (!person.getEmail().isEmpty()) {
         System.out.println("  E-mail address: " + person.getEmail());
+
       }

       for (Person.PhoneNumber phoneNumber : person.getPhonesList()) {

Can you please suggest how to run tests and regenerate outputs?

If I run make tests on the base revision, it returns pages and pages of errors.

If I run

./build/fbcode_builder/getdeps.py test --allow-system-packages --src-dir=. --num-jobs=64 sapling

as suggested in readme, it reports

project sapling has not been built

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. (Because this pull request was imported automatically, there will not be any future comments.)

@quark-zju
Copy link
Contributor

If you built sl under eden/scm, you can use run-tests.py -i under eden/scm/tests to run and update tests.

This might be a breaking change for automation (for example, something uses a regex ...@@$ to parse the line number). It's safer to only apply the behavior change for non-automation use-cases. You can use ui.plain("diff-showfunction") to test if it's automation.

@stepancheg
Copy link
Contributor Author

stepancheg commented Feb 14, 2025

@quark-zju it is broken too (or requires some setup I'm not aware of). The same pages of errors like:

$ tests/run-tests.py -i

...

 Don't hide commits when fbcodereview.hide-landed-commits=false:
   $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg debugmarklanded --verbose --dry-run --config fbcodereview.hide-landed-commits=false
-  marking D123 (a421db7622bf, d131c2d7408a) as landed as 23bffadc9066
-  marking D456 (524f3ad51d24) as abandoned
-  marked 2 commits as landed
-  marked 1 commit as abandoned
-  (this is a dry-run, nothing was actually done)
+  abort: '$TESTTMP/repo1' is not inside a repository, but this command requires a repository!
+  (use 'cd' to go to a directory inside a repository and try again)
+  [255]

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

Successfully merging this pull request may close these issues.

3 participants