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

Consider renaming fail_map to error_map #1446

Closed
mre opened this issue Jun 14, 2024 · 4 comments · Fixed by #1560
Closed

Consider renaming fail_map to error_map #1446

mre opened this issue Jun 14, 2024 · 4 comments · Fixed by #1560
Labels
breaking-change bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mre
Copy link
Member

mre commented Jun 14, 2024

We no longer use the term failure in our system; we now refer to it as error. Therefore, it makes sense to rename fail_map to error_map for consistency. Similar to #1443, this will introduce a breaking change, but since we haven't reached a stable version yet, I want to address these naming inconsistencies beforehand.

@mre
Copy link
Member Author

mre commented Jun 14, 2024

Related discussion, which could be tackled at the same time: #1013

@thomas-zahner
Copy link
Member

I totally agree with that change 👍

As for #1013 this was actually already implemented in 250f7a8. Although, I think that the text field is a bit verbose, maybe we could reduce to just the HTTP status text.

The current JSON output looks like:

  "success_map": {
    "http://www.example.com/": [
      {
        "url": "https://www.iana.org/domains/example",
        "status": {
          "text": "OK (200 OK)",
          "code": 200
        }
      }
    ]
  },

@mre
Copy link
Member Author

mre commented Jun 24, 2024

Yup, sounds like a good idea!

@mre mre added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed breaking-change and removed waiting-for-feedback labels Sep 27, 2024
mre added a commit that referenced this issue Nov 7, 2024
@mre
Copy link
Member Author

mre commented Nov 7, 2024

I took a stab at it in #1560.
The status text has already been fixed with #1553. It looks a little cleaner now:

"success_map": {
  "http://www.example.com/": [
    {
      "url": "https://www.iana.org/domains/example",
      "status": {
        "text": "200 OK",
        "code": 200
      }
    }
  ]
}

@mre mre closed this as completed in #1560 Nov 8, 2024
@mre mre closed this as completed in 9dc4217 Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants