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

Update the out-of-date readme file #72

Merged
merged 13 commits into from
Apr 26, 2024

Conversation

chanchiwai-ray
Copy link
Contributor

@chanchiwai-ray chanchiwai-ray commented Apr 25, 2024

  • Remove envdir from tox.ini as it was never intended to be used like that
  • Update existing README.md
  • Create a README-SNAP.md for snap.
  • Fixed broken snap build
  • Fixed typo in class name

Copy link
Contributor

@jneo8 jneo8 left a comment

Choose a reason for hiding this comment

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

Small typo, others LGTM.

prometheus_hardware_exporter/exporter.py Outdated Show resolved Hide resolved
@chanchiwai-ray chanchiwai-ray requested a review from jneo8 April 25, 2024 09:12
@sudeephb
Copy link
Contributor

Doesn't it make more sense to put README-SNAP.md inside the snap directory?

tox.ini Show resolved Hide resolved
@chanchiwai-ray chanchiwai-ray requested a review from rgildein April 25, 2024 10:55
@chanchiwai-ray
Copy link
Contributor Author

chanchiwai-ray commented Apr 25, 2024

Doesn't it make more sense to put README-SNAP.md inside the snap directory?

Yes, but snapcraft will treat it as part of the snap build, and give you warning when building it. I.e.

Building the snap
The 'snap' directory is meant specifically for snapcraft, but it contains
the following non-snapcraft-related paths:
- README-SNAP.md

This is unsupported and may cause unexpected behavior. If you must store
these files within the 'snap' directory, move them to 'snap/local'
which is ignored by snapcraft.                                             
    

Copy link
Contributor

@dashmage dashmage left a comment

Choose a reason for hiding this comment

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

I've added some minor comments, none which are blocking. Can't believe we haven't addressed many of the typos and other errors for so long so thanks for creating this PR 👍

README-SNAP.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@chanchiwai-ray chanchiwai-ray requested a review from dashmage April 26, 2024 02:14
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@chanchiwai-ray chanchiwai-ray requested a review from Pjack April 26, 2024 02:58
@chanchiwai-ray chanchiwai-ray merged commit 2b7c315 into canonical:main Apr 26, 2024
3 checks passed
@chanchiwai-ray chanchiwai-ray deleted the chores/update-docs branch April 26, 2024 07:20
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.

6 participants