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

Update for 2023 #45

Merged
merged 13 commits into from
Jul 21, 2023
Merged

Update for 2023 #45

merged 13 commits into from
Jul 21, 2023

Conversation

sebastian-berlin-wmse
Copy link
Member

Based on changes made in project structure for 2023, including:

  • Program information is now extracted from the project file rather
    than a wikipage. Strategies are no longer included.
  • Program overview page is no longer created.
  • The projects year page now only has headers for programs and
    doesn't include the numbers.
  • The config used by WMSE is now included as default.

Copy link
Member

@lokal-profil lokal-profil left a comment

Choose a reason for hiding this comment

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

Wiki.py I still need to check on a proper screen

README.md Show resolved Hide resolved
config.yaml Show resolved Hide resolved
@lokal-profil
Copy link
Member

lokal-profil commented May 31, 2023

Should the Readme explivitly list the expected .env variables?

One is implicitly listed but it is not clear if it is the only one

Copy link
Member

@lokal-profil lokal-profil left a comment

Choose a reason for hiding this comment

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

I've looked through the code and left some comments. haven't tried to spin it up yet though

wiki.py Show resolved Hide resolved
@sebastian-berlin-wmse
Copy link
Member Author

Should the Readme explivitly list the expected .env variables?

One is implicitly listed but it is not clear if it is the only one

I guess that it may be helpful to list env variables if there are a lot of them. Now there's only one and it's mentioned where it's relevant.

@lokal-profil
Copy link
Member

Should the Readme explivitly list the expected .env variables?
One is implicitly listed but it is not clear if it is the only one

I guess that it may be helpful to list env variables if there are a lot of them. Now there's only one and it's mentioned where it's relevant.

Fair enough.

Does the program still work fine if the env variable is provided directly but the .env file is missing?
Does it fail gracefully (or at least in an informative way) if the env variable cannot be found? Since it's more likely to be missing now than when it was in the config.

@sebastian-berlin-wmse
Copy link
Member Author

Does the program still work fine if the env variable is provided directly but the .env file is missing?

Yes. You can use either. If you have both the value from the command line take precedence.

Does it fail gracefully (or at least in an informative way) if the env variable cannot be found? Since it's more likely to be missing now than when it was in the config.

It raises an exception with the error from Phab's API. It looks like this:

[...]
phab.PhabApiError: Error from Phabricator API: Session key is not present.
CRITICAL: Exiting due to uncaught exception <class 'phab.PhabApiError'>
2023-06-19 16:18:35,715 [CRITICAL] (logging): Exiting due to uncaught exception <class 'phab.PhabApiError'>

@lokal-profil
Copy link
Member

@sebastian-berlin-wmse the question about the config.yml is still unresolved

My comment is about the removal of Globala mätetal <YEAR>: Globala mätetal

@lokal-profil
Copy link
Member

@sebastian-berlin-wmse Do you want to check that this still works with #48 (as per your comments there). After that (and the resulting rebase) it should be good to merge.

@sebastian-berlin-wmse sebastian-berlin-wmse merged commit 9c40a81 into master Jul 21, 2023
@lokal-profil lokal-profil deleted the 2023 branch July 24, 2023 07:03
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.

None yet

2 participants