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.

Sign up for GitHub

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

Jump to bottom

✨ NEW: Basic addition of core. prefix #3

Merged
merged 5 commits into from
Mar 10, 2022
Merged

✨ NEW: Basic addition of core. prefix #3

merged 5 commits into from
Mar 10, 2022

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented Mar 10, 2022

Add some basic refactoring code for adding the core. prefix to the
deprecated aiida-core entry points for:

  • Loading a node type using one of the factories
  • Full entry points strings, including both the group and deprecated
    entry point separated by a colon, e.g.:

'aiida.calculations:arithmetic.add'

Also add basic CLI functionality for using aiida-upgrade.

@mbercx mbercx requested a review from chrisjsewell March 10, 2022 01:32
@mbercx
✨ NEW: Basic addition of core. prefix
fbfdadc 
Add some basic refactoring code for adding the `core.` prefix to the
deprecated `aiida-core` entry points for:

* Loading a node type using one of the factories
* Full entry points strings, including both the group and deprecated
entry point separated by a colon, e.g.:

'aiida.calculations:arithmetic.add'

Also add basic CLI functionality for using `aiida-upgrade`.
@mbercx
Copy link
Member Author

mbercx commented Mar 10, 2022

@chrisjsewell just a basic version for now, using libcst. bowler seems to be no longer under active development currently, and they actually recommended using LibCST. It's true that the visitors are quite verbose, but in the end I think we have a lot more flexibility using LibCST.

The package can simply be used on any path:

aiida-upgrade <path>

To recursively update all files in that <path>. Not sure if you had another CLI in mind? I'll write the README in the morning. 🥱

@chrisjsewell
Copy link
Member

👌 maybe if you could add at least a very basic test?

@mbercx
Copy link
Member Author

mbercx commented Mar 10, 2022

@chrisjsewell I also quickly updated the README with some more instructions and a list of migrations that are supported, and those that aren't (yet).

@mbercx mbercx mentioned this pull request Mar 10, 2022
Merged
sphuber
sphuber reviewed Mar 10, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
.github/ci.yml Outdated
@@ -0,0 +1,37 @@
name: ci
Copy link

Choose a reason for hiding this comment

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

I think for this to work it has to be in the .github/workflows folder.

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops! It was late 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess this will only run for the next PR, after this is merged, right?

README.md Outdated Show resolved Hide resolved
@mbercx mbercx merged commit be64a11 into aiidateam:main Mar 10, 2022
@mbercx mbercx deleted the new/core-prefix branch March 10, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sphuber sphuber sphuber approved these changes

@chrisjsewell chrisjsewell Awaiting requested review from chrisjsewell

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mbercx @chrisjsewell @sphuber