Skip to content

Conversation

@purajit
Copy link
Contributor

@purajit purajit commented Dec 30, 2024

Summary

Changes two things about the entry:

  • make the example valid TOML - inline tables must be a single line, at least till v1.1.0 is released,
    but also while in the future the toml version used by ruff might handle it, it would probably be
    good to stick to a spec that's readable by the vast majority of other tools and versions as well,
    especially if people are using pyproject.toml. The current example leads to ruff failure.
    See Allow newlines and trailing comma in inline tables toml-lang/toml#904
  • adds a line about the ability to add non-Python files to the map, which I think is a specific and
    important feature people should know about (in fact, I would assume this could potentially
    become the single biggest use-case for this).

Test Plan

Ran doc creation as described in the contribution guide.

@purajit purajit changed the title [docs] improve and fix entry for analyze.include-dependencies [docs] improve and fix entry for analyze.include-dependencies Dec 30, 2024
@purajit
Copy link
Contributor Author

purajit commented Dec 30, 2024

new entry
image

@github-actions
Copy link
Contributor

github-actions bot commented Dec 30, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added the documentation Improvements or additions to documentation label Dec 30, 2024
@purajit
Copy link
Contributor Author

purajit commented Dec 30, 2024

For the second point, I was trying to see if that was intentional or not; couldn't find anything
in the original PR, and the code

if let Some((root, globs)) = include_dependencies {
let mut glob_resolver = glob_resolver.lock().unwrap();
imports.extend(glob_resolver.resolve(root, globs));
}

doesn't suggest anything strongly either; GlobResolver resolves all files. My main hesitation
is that it's consistently called an "import map", and there are already issues/discussions about
expanding the scope of analyze graph assuming that these are only Python deps.. I hope it
stays, though I understand if it won't.

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks! It's an intentional behavior.

@charliermarsh charliermarsh enabled auto-merge (squash) December 30, 2024 15:41
@charliermarsh charliermarsh merged commit 42cc67a into astral-sh:main Dec 30, 2024
20 checks passed
@purajit purajit deleted the 20241230-include-dependencies-docs branch December 31, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants