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

ci: add build-wheel job #812

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

afuetterer
Copy link
Member

@afuetterer afuetterer commented Nov 3, 2023

Description

I added a CI job build-wheel, that runs on each pull request.

The job will build the frontend bundle using webpack and then build a python wheel.

The wheel will be renamed from the default name rdmo-{version}-py3-none-any.whl (e.g. rdmo-2.0.2-py3-none-any.whl) to rdmo-{version}-pr{pr-number}-py3-none-any.whl (e.g. rdmo-2.0.2-pr-802-py3-none-any.whl). This enables developers to install the wheel locally or on their test servers and then test the changes of a PR. The renaming makes it easier to tell the difference between wheels from different PRs.

Is the renaming reasonable or should it I remove this step?

Related issue: #802

Types of Changes

  • Build related changes

Checklist

  • I have read the contributor guide.
  • My code follows the code style of this project.
  • All new and existing tests passed.

@coveralls
Copy link

Coverage Status

coverage: 80.917%. remained the same
when pulling c6ef0b5 on afuetterer:802-build-wheel-in-ci
into a54867e on rdmorganiser:master.

@afuetterer
Copy link
Member Author

@afuetterer
Copy link
Member Author

Does this work for you, @MyPyDavid? Can you try to install the wheel from the uploaded artifact?

@MyPyDavid
Copy link
Member

MyPyDavid commented Nov 16, 2023

Not directly, the filename needs to follow a certain pattern.

Wheel files typically follow the pattern {distribution}-{version}(-{build tag})?-{python tag}-{abi tag}-{platform tag}.whl.

Here's an example of a valid wheel filename: example_pkg-0.0.1-py3-none-any.whl.

I cant really find an easy explanation for this.

Maybe just keep a simple filename with that pattern?

@afuetterer

This comment was marked as resolved.

@MyPyDavid

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@afuetterer afuetterer changed the base branch from master to dev-2.1.0 November 16, 2023 14:12
@afuetterer

This comment was marked as resolved.

@MyPyDavid
Copy link
Member

nice!
Here are some infos for downloading artifacts via the CLI, actions/upload-artifact#89.
I would include a download via shell script in my rdmo-app for this for example.

We could add it to rdmo-docs-en eventually, to include it in dev setup docs as a pre-release or feature-preview set up right?

@afuetterer

This comment was marked as resolved.

@MyPyDavid
Copy link
Member

yes, I could install it and it runs!

eg. pip install rdmo-2.0.2dev812-py3-none-any.whl'[allauth]'

@jochenklar
Copy link
Member

jochenklar commented Nov 16, 2023

@afuetterer
Copy link
Member Author

Where do you want the +?

Please give me the desired wheel name and I will adapt the rename step.

@jochenklar
Copy link
Member

Sorry, my suggestion would be rdmo-2.0.2+pr812-py3-none-any.whl which I guess is conformal with the local version identifiers in the linked document. They are however designed to be used for downstream packages, like a distro specific version of e.g. requests or so. I am not super sure if it is ok to use it for this purpose here.

Local version identifiers SHOULD NOT be used when publishing upstream projects to a public index server, but MAY be used to identify private builds created directly from the project source. Local version identifiers SHOULD be used by downstream projects when releasing a version that is API compatible with the version of the upstream project identified by the public version identifier, but contains additional changes (such as bug fixes). As the Python Package Index is intended solely for indexing and hosting upstream projects, it MUST NOT allow the use of local version identifiers.

I would also be ok with just the public version.

@jochenklar
Copy link
Member

ok after reading this:

While they may be useful for continuous integration purposes, publishing developmental releases of pre-releases to general purpose public index servers is strongly discouraged, as it makes the version identifier difficult to parse for human readers. If such a release needs to be published, it is substantially clearer to instead create a new pre-release by incrementing the numeric component.

Developmental releases of post-releases are also strongly discouraged, but they may be appropriate for projects which use the post-release notation for full maintenance releases which may include code changes.

I think dev<pr> is much better. We can merge here (I guess).

@afuetterer
Copy link
Member Author

Does the name work for you @MyPyDavid? Then I would merge the PR after your approval.

Copy link
Member

@MyPyDavid MyPyDavid left a comment

Choose a reason for hiding this comment

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

yes for the name with dev in it

@afuetterer afuetterer merged commit 5304eb2 into rdmorganiser:dev-2.1.0 Nov 21, 2023
17 checks passed
@afuetterer afuetterer deleted the 802-build-wheel-in-ci branch November 21, 2023 08:34
@MyPyDavid MyPyDavid added this to the 2.1.0 milestone Nov 21, 2023
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.

4 participants