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

Add util to get consolidated page_config from higher levels #448

Merged
merged 7 commits into from
Apr 22, 2024
Merged

Add util to get consolidated page_config from higher levels #448

merged 7 commits into from
Apr 22, 2024

Conversation

divyansshhh
Copy link
Contributor

Copy link

welcome bot commented Apr 19, 2024

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@krassowski
Copy link
Member

The linter is not happy yet:

jupyterlab_server/config.py: note: In function "_get_config_manager":
jupyterlab_server/config.py:396: error: "object" has no attribute "__iter__";
maybe "__dir__" or "__str__"? (not iterable)  [attr-defined]
            for p in paths[_level]:
                     ^~~~~~~~~~~~~
jupyterlab_server/config.py:398: error: Left operand of "and" is always true 
[redundant-expr]
            if write_config_dir is None and paths[_level]:
               ^~~~~~~~~~~~~~~~~~~~~~~~
jupyterlab_server/config.py:399: error: Value of type "object" is not indexable
 [index]
                write_config_dir = osp.join(paths[_level][0], config_name)
                                            ^~~~~~~~~~~~~~~~

Divyansh Choudhary and others added 2 commits April 20, 2024 19:38
@divyansshhh
Copy link
Contributor Author

I think lint is fixed now.

Comment on lines 388 to 391
if include_higher_levels:
levels = allowed[allowed.index(level) :]
else:
config_dir = SYSTEM_CONFIG_PATH[0]
levels = [level]
Copy link
Member

Choose a reason for hiding this comment

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

Still failing here:

jupyterlab_server/config.py:388:5: SIM108 Use ternary operator `levels = allowed[allowed.index(level):] if include_higher_levels else [level]` instead of `if`-`else`-block
Found 1 error.

IMO that's too pedantinc, but linter is a linter.

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

LGTM thank you @divyansshhh =!

@krassowski krassowski merged commit 951b54c into jupyterlab:main Apr 22, 2024
27 checks passed
Copy link

welcome bot commented Apr 22, 2024

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@krassowski
Copy link
Member

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.

2 participants