Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

MNT: Remove unused AddViewerMessage #939

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Oct 15, 2021

Description

AddViewerMessage is not used. Its existence is actually confusing because there is a NewViewerMessage that is actually used and it sounds like the same thing.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
  • Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
  • Do the proposed changes follow the STScI Style Guides?
  • Are tests added/updated as required? If so, do they follow the STScI Style Guides?
  • Are docs added/updated as required? If so, do they follow the STScI Style Guides?
  • Did the CI pass? If not, are the failures related?
  • Is a change log needed? If yes, is it added to CHANGES.rst?
  • Is a milestone set? Milestone is only currently required for PRs related to Imviz MVP.
  • After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

@pllim pllim added this to the 2.1 milestone Oct 15, 2021
@github-actions github-actions bot added the documentation Explanation of code and concepts label Oct 15, 2021
@pllim pllim requested a review from havok2063 October 15, 2021 21:22
@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #939 (39eded8) into main (43976a1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #939      +/-   ##
==========================================
+ Coverage   69.38%   69.40%   +0.01%     
==========================================
  Files          69       69              
  Lines        4887     4880       -7     
==========================================
- Hits         3391     3387       -4     
+ Misses       1496     1493       -3     
Impacted Files Coverage Δ
jdaviz/core/events.py 85.49% <100.00%> (+1.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43976a1...39eded8. Read the comment docs.

Copy link
Collaborator

@havok2063 havok2063 left a comment

Choose a reason for hiding this comment

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

Works for me!

Copy link
Member

@kecnry kecnry left a comment

Choose a reason for hiding this comment

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

Sure enough, I don't see any other mention in the code, and no good reason to keep it around (we can always add it back later).

@kecnry kecnry merged commit 3d148c2 into spacetelescope:main Oct 18, 2021
@pllim pllim deleted the remove-addviewermessage branch October 18, 2021 20:47
@pllim pllim mentioned this pull request Feb 3, 2022
16 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api API change documentation Explanation of code and concepts Ready for final review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants