Skip to content

Conversation

@superplay1
Copy link
Collaborator

why: cleaner structure
what: restructured model,chip,task
how: moved model.py one layer up, moved chip.py, tasks.py in libraries and modified imports

@CLAassistant
Copy link

CLAassistant commented May 20, 2021

CLA assistant check
All committers have signed the CLA.

@superplay1 superplay1 requested review from lazyoracle and nwittler May 20, 2021 10:05
@lazyoracle lazyoracle added this to the 1.3 milestone May 20, 2021
@fedroy
Copy link
Collaborator

fedroy commented May 21, 2021

@superplay1 Please provide a regex replace command to update the script files.
@lazyoracle Is there a way to in some way "pin" these commands so that one is warned when running the code.
It would be optimal is when going from version X to version Y one is prompted with all the script modifications needed to make their scripts be compatible again. Does such a feature exist anywhere?

@lazyoracle
Copy link
Member

lazyoracle commented May 21, 2021

@lazyoracle Is there a way to in some way "pin" these commands so that one is warned when running the code.
It would be optimal is when going from version X to version Y one is prompted with all the script modifications needed to make their scripts be compatible again. Does such a feature exist anywhere?

The standard practice is to use deprecation warnings and not make API breaking changes without a buffer release in between. For example, if we are now in 1.2.2 and thinking of making breaking changes, we should make a release 1.2.3 which includes deprecation warnings for all the parts of the codebase that we are planning to break, so that anybody using version 1.2.3 gets notified about where their scripts will break. This is typically also included in the CHANGELOG as discussed in #88. The breaking changes are then only made in 1.2.4 where as a best practice there should be error handling code that catches code which still tries to use the previous API and throws an error pointing them to the documentation of the updated API or instructions on how to upgrade their scripts.

@fedroy
Copy link
Collaborator

fedroy commented May 21, 2021

Should we use this PR as an example to implement the best practices you just described?

@picbeats
Copy link
Collaborator

picbeats commented May 21, 2021

The standard practice is to use deprecation warnings and not make API breaking changes without a buffer release in between.

Semver suggests that after a public API is released (Ver >= 1.x) API breaking changes should trigger major version increment. i.e. 1.2.2 -> 2.0.0-dev

@lazyoracle
Copy link
Member

after a public API is released (Ver >= 1.x)

From what I understand, for all practical purposes our current 1.x release is more of a "private beta" and not a stable public API in the usual sense. As such API breaking changes should be fine until we move to a stage where we can call this stable and have a 2.0 release

Copy link
Collaborator

@nwittler nwittler left a comment

Choose a reason for hiding this comment

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

Looks good. Just provide the requested search replace expressions, e.g.
c3.system.model -> c3.model

Copy link
Member

@lazyoracle lazyoracle left a comment

Choose a reason for hiding this comment

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

Everything looks good. Probably need to provide the search replace command as requested by @fedroy. You can just add it to the description of this PR.

@lazyoracle lazyoracle merged commit 8369d14 into q-optimize:dev May 26, 2021
@lazyoracle lazyoracle added the API-change Breaks API label Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API-change Breaks API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants