Skip to content

Conversation

@cbielow
Copy link
Contributor

@cbielow cbielow commented Feb 17, 2022

Description

This PR is a first attempt to separate the plotting of layer data in Canvas1D from the layer type.
Painting is delegated to a separate class for the essential parts.
This will allow to add painters for chromatograms, mobilograms etc in the future (some more refatoring is needed).

Going in small pieces here, so this is the first PR or some more to come.
The Painter class is not fully fledged out yet. Expect some future changes there.

How can I get additional information on failed tests during CI:

If your PR is failing you can check out

Note:

  • Once you opened a PR try to minimize the number of pushes to it as every push will trigger CI (automated builds and test) and is rather heavy on our infrastructure (e.g., if several pushes per day are performed).

Advanced commands (admins / reviewer only):

  • /rebase will try to rebase the PR on the current develop branch.
  • /reformat (experimental) applies the clang-format style changes as additional commit
  • setting the label NoJenkins will skip tests for this PR on jenkins (saves resources e.g., on edits that do not affect tests)

@jpfeuffer
Copy link
Contributor

Should the 1D Painter also be responsible for drawing the ion ladder?

@timosachsenberg
Copy link
Contributor

Should the 1D Painter also be responsible for drawing the ion ladder?

I think even cleaner would be putting the ion ladder into a separate widget e.g. at the top.

@cbielow
Copy link
Contributor Author

cbielow commented Feb 18, 2022

Should the 1D Painter also be responsible for drawing the ion ladder?

I think even cleaner would be putting the ion ladder into a separate widget e.g. at the top.

yes indeed. Its on the list, but one thing at a time :)

@cbielow cbielow merged commit dc65927 into OpenMS:develop Feb 18, 2022
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