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

Refactor project specific code #113

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

soehms
Copy link
Member

@soehms soehms commented Jan 12, 2023

@kwankyu, @mkoeppe I'm sorry I haven't found time to help here for a few weeks. While trying to update myself (at least a bit), I noticed that in the heat of development, a lot of Sage-specific code ended up in the migration.py file.

(sage-sh) sebastian@ThinkPadKlein:trac-to-github$ grep -ci ^[^\#]*sage migrate.py
102

So I'm proposing this PR to relegate this to the config files. The patch produces exactly the same output as the previous commit, but reduces the Sage-specific coding in the Python file:

(sage-sh) sebastian@ThinkPadKlein:trac-to-github$ grep -i ^[^\#]*sage migrate.py
Modified and extended for the migration of SageMath from Trac to GitHub.
            Decode title prefixes such as [with patch, positive review] used in early Sage tickets.
    FORMAT = "%(message)s"

Furthermore, I do some cleaning in terms of avoidable use of literals.

BTW: I noticed this unknown milestone "sage-9.1.1" (with and without the patch):

           INFO     Migrating ticket #25316 ( 58 changes): "Replace python 5755 patch by monkey patch"                                                                                                                                                                                                                                                      migrate.py:1853
           WARNING  Ignoring unknown milestone "sage-9.1.1"                                                                                                                                                                                                                                                                                                 migrate.py:2194
           WARNING  Ignoring unknown milestone "sage-9.1.1"                                                                                                                                                                                                                                                                                                 migrate.py:2187
           INFO     Migrating ticket #25317 ( 20 changes): "Special-case pol*term, term*pol for generic polyno"                                                                                                                                                                                                                                             migrate.py:1853
....
   Unmapped milestones
┏━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃  Milestone ┃ Frequency ┃
┡━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ sage-9.1.1 │ 18        │
└────────────┴───────────┘

It is not present in the unmapped_milestones.txt. Where does it come from? I didn't find it in any Trac ticket.

The following questions go back in time, but I couldn't find answers for them:

  1. I'm wondering why renaming WikiStart to Home was dropped as it breaks the start page for the GitHub wiki (assuming it will be chosen as the final destination). What is the intention? Will it be replaced by a softlink?

  2. I still don't understand the need to delete the wiki subtree. I agree with the argument given in the comment dated 12/22/16 of PR Fix E5 of issue #18 #26 that there could be ambiguities (theoretically). But I don't see any examples of this. Anyway, if this is necessary, why are we using whitespaces as delimiters in filenames?

@mkoeppe
Copy link

mkoeppe commented Jan 12, 2023

Thanks @soehms, these are very welcome cleanups. I'll test later today.

@mkoeppe
Copy link

mkoeppe commented Jan 12, 2023

BTW: I noticed this unknown milestone "sage-9.1.1" (with and without the patch):

This milestone was used temporarily for a potential bugfix release, but later deleted.
Not sure what to do with it. I think we can deal with it in the same way as we can do with component labels that have fallen out of use (#99 (comment)): Create the milestone during the migration, then later delete it in the GitHub UI.

@soehms
Copy link
Member Author

soehms commented Jan 12, 2023

BTW: I noticed this unknown milestone "sage-9.1.1" (with and without the patch):

This milestone was used temporarily for a potential bugfix release, but later deleted. Not sure what to do with it. I think we can deal with it in the same way as we can do with component labels that have fallen out of use (#99 (comment)): Create the milestone during the migration, then later delete it in the GitHub UI.

Thanks! I will have a look at the referenced issue.

@kwankyu
Copy link

kwankyu commented Jan 12, 2023

  1. I'm wondering why renaming WikiStart to Home was dropped as it breaks the start page for the GitHub wiki (assuming it will be chosen as the final destination). What is the intention? Will it be replaced by a softlink?

"Home" page is special, and is one level up to the other pages. This makes it difficult to write correct paths (to attachments or to other wiki pages) (if I remember correctly).

On the other hand, I think it's good to start a new "Home" page for the new wiki on GitHub.

... why are we using whitespaces as delimiters in filenames?

I tried with others, but ended in the whitespace. The others didn't work well (for me).

After migration, we can edit and reorganize the pages. So I don't think it's a big deal.

@mkoeppe mkoeppe merged commit 4bc1db7 into sagemath:master Jan 12, 2023
@mkoeppe
Copy link

mkoeppe commented Jan 12, 2023

This is working well. Thanks again!

@soehms soehms deleted the refactor_project_code branch January 13, 2023 07:12
@soehms
Copy link
Member Author

soehms commented Jan 13, 2023

This is working well. Thanks again!

Thanks, too!

@soehms
Copy link
Member Author

soehms commented Jan 13, 2023

  1. I'm wondering why renaming WikiStart to Home was dropped as it breaks the start page for the GitHub wiki (assuming it will be chosen as the final destination). What is the intention? Will it be replaced by a softlink?

"Home" page is special, and is one level up to the other pages. This makes it difficult to write correct paths (to attachments or to other wiki pages) (if I remember correctly).

On the other hand, I think it's good to start a new "Home" page for the new wiki on GitHub.

... why are we using whitespaces as delimiters in filenames?

I tried with others, but ended in the whitespace. The others didn't work well (for me).

After migration, we can edit and reorganize the pages. So I don't think it's a big deal.

I see! Thanks for the answer!

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.

3 participants